Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] PCI: hv: support reporting serial number as slot information
From: Stephen Hemminger @ 2018-08-29 16:24 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev, linux-pci
In-Reply-To: <20180829162452.25805-1-sthemmin@microsoft.com>

The Hyper-V host API for PCI provides a unique "serial number" which
can be used as basis for sysfs PCI slot table. This can be useful
for cases where userspace wants to find the PCI device based on
serial number.

When an SR-IOV NIC is added, the host sends an attach message
with serial number. The kernel doesn't use the serial number, but
it is useful when doing the same thing in a userspace driver such
as the DPDK. By having /sys/bus/pci/slots/N it provides a direct
way to find the matching PCI device.

There may be some cases where serial number is not unique such
as when using GPU's. But the PCI slot infrastructure will handle
that by adding suffix "2-1" etc.

This has a side effect which may also be useful. The common udev
network device naming policy uses the slot information (rather
than PCI address). This causes udev to give shorter network device
names for VF devices.  It does not break applications or startup
because the VF device must never be configured directly.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 30 +++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index c00f82cc54aa..e6a6c1146a41 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -89,6 +89,8 @@ static enum pci_protocol_version_t pci_protocol_version;
 
 #define STATUS_REVISION_MISMATCH 0xC0000059
 
+#define SLOT_NAME_SIZE 21
+
 /*
  * Message Types
  */
@@ -494,6 +496,7 @@ struct hv_pci_dev {
 	struct list_head list_entry;
 	refcount_t refs;
 	enum hv_pcichild_state state;
+	struct pci_slot *pci_slot;
 	struct pci_function_description desc;
 	bool reported_missing;
 	struct hv_pcibus_device *hbus;
@@ -1457,6 +1460,28 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus)
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 }
 
+static void hv_pci_assign_slots(struct hv_pcibus_device *hbus)
+{
+	struct hv_pci_dev *hpdev;
+	char name[SLOT_NAME_SIZE];
+	unsigned long flags;
+	int slot_nr;
+
+	spin_lock_irqsave(&hbus->device_list_lock, flags);
+	list_for_each_entry(hpdev, &hbus->children, list_entry) {
+		if (hpdev->pci_slot)
+			continue;
+
+		slot_nr = PCI_SLOT(wslot_to_devfn(hpdev->desc.win_slot.slot));
+		snprintf(name, SLOT_NAME_SIZE, "%u", hpdev->desc.ser);
+		hpdev->pci_slot = pci_create_slot(hbus->pci_bus, slot_nr,
+					  name, NULL);
+		if (!hpdev->pci_slot)
+			pr_warn("pci_create slot %s failed\n", name);
+	}
+	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
+}
+
 /**
  * create_root_hv_pci_bus() - Expose a new root PCI bus
  * @hbus:	Root PCI bus, as understood by this driver
@@ -1480,6 +1505,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
 	pci_lock_rescan_remove();
 	pci_scan_child_bus(hbus->pci_bus);
 	pci_bus_assign_resources(hbus->pci_bus);
+	hv_pci_assign_slots(hbus);
 	pci_bus_add_devices(hbus->pci_bus);
 	pci_unlock_rescan_remove();
 	hbus->state = hv_pcibus_installed;
@@ -1742,6 +1768,7 @@ static void pci_devices_present_work(struct work_struct *work)
 		 */
 		pci_lock_rescan_remove();
 		pci_scan_child_bus(hbus->pci_bus);
+		hv_pci_assign_slots(hbus);
 		pci_unlock_rescan_remove();
 		break;
 
@@ -1858,6 +1885,9 @@ static void hv_eject_device_work(struct work_struct *work)
 	list_del(&hpdev->list_entry);
 	spin_unlock_irqrestore(&hpdev->hbus->device_list_lock, flags);
 
+	if (hpdev->pci_slot)
+		pci_destroy_slot(hpdev->pci_slot);
+
 	memset(&ctxt, 0, sizeof(ctxt));
 	ejct_pkt = (struct pci_eject_response *)&ctxt.pkt.message;
 	ejct_pkt->message_type.type = PCI_EJECTION_COMPLETE;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 2/2] hv_netvsc: pair VF based on serial number
From: Stephen Hemminger @ 2018-08-29 16:24 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev, linux-pci
In-Reply-To: <20180829162452.25805-1-sthemmin@microsoft.com>

Matching network device based on MAC address is problematic
since a non-VF network device can be created with a duplicate MAC
address causing confusion and problems.  The VMBus API provides
a serial number that is a better matching method.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc.c     |  3 ++
 drivers/net/hyperv/netvsc_drv.c | 58 +++++++++++++++++++--------------
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 31c3d77b4733..fe01e141c8f8 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1203,6 +1203,9 @@ static void netvsc_send_vf(struct net_device *ndev,
 
 	net_device_ctx->vf_alloc = nvmsg->msg.v4_msg.vf_assoc.allocated;
 	net_device_ctx->vf_serial = nvmsg->msg.v4_msg.vf_assoc.serial;
+	netdev_info(ndev, "VF slot %u %s\n",
+		    net_device_ctx->vf_serial,
+		    net_device_ctx->vf_alloc ? "added" : "removed");
 }
 
 static  void netvsc_receive_inband(struct net_device *ndev,
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1121a1ec407c..9dedc1463e88 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1894,20 +1894,6 @@ static void netvsc_link_change(struct work_struct *w)
 	rtnl_unlock();
 }
 
-static struct net_device *get_netvsc_bymac(const u8 *mac)
-{
-	struct net_device_context *ndev_ctx;
-
-	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
-		struct net_device *dev = hv_get_drvdata(ndev_ctx->device_ctx);
-
-		if (ether_addr_equal(mac, dev->perm_addr))
-			return dev;
-	}
-
-	return NULL;
-}
-
 static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
 {
 	struct net_device_context *net_device_ctx;
@@ -2036,26 +2022,48 @@ static void netvsc_vf_setup(struct work_struct *w)
 	rtnl_unlock();
 }
 
+/* Find netvsc by VMBus serial number.
+ * The PCI hyperv controller records the serial number as the slot.
+ */
+static struct net_device *get_netvsc_byslot(const struct net_device *vf_netdev)
+{
+	struct device *parent = vf_netdev->dev.parent;
+	struct net_device_context *ndev_ctx;
+	struct pci_dev *pdev;
+
+	if (!parent || !dev_is_pci(parent))
+		return NULL; /* not a PCI device */
+
+	pdev = to_pci_dev(parent);
+	if (!pdev->slot) {
+		netdev_notice(vf_netdev, "no PCI slot information\n");
+		return NULL;
+	}
+
+	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
+		if (!ndev_ctx->vf_alloc)
+			continue;
+
+		if (ndev_ctx->vf_serial == pdev->slot->number)
+			return hv_get_drvdata(ndev_ctx->device_ctx);
+	}
+
+	netdev_notice(vf_netdev,
+		      "no netdev found for slot %u\n", pdev->slot->number);
+	return NULL;
+}
+
 static int netvsc_register_vf(struct net_device *vf_netdev)
 {
-	struct net_device *ndev;
 	struct net_device_context *net_device_ctx;
-	struct device *pdev = vf_netdev->dev.parent;
 	struct netvsc_device *netvsc_dev;
+	struct net_device *ndev;
 	int ret;
 
 	if (vf_netdev->addr_len != ETH_ALEN)
 		return NOTIFY_DONE;
 
-	if (!pdev || !dev_is_pci(pdev) || dev_is_pf(pdev))
-		return NOTIFY_DONE;
-
-	/*
-	 * We will use the MAC address to locate the synthetic interface to
-	 * associate with the VF interface. If we don't find a matching
-	 * synthetic interface, move on.
-	 */
-	ndev = get_netvsc_bymac(vf_netdev->perm_addr);
+	ndev = get_netvsc_byslot(vf_netdev);
 	if (!ndev)
 		return NOTIFY_DONE;
 
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH 0/4] clk-pmc-atom + r8169: Add ether_clk handling to fix suspend issues
From: Andy Shevchenko @ 2018-08-29 16:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: David S . Miller, Heiner Kallweit, Michael Turquette,
	Stephen Boyd, Irina Tirdea, netdev, Johannes Stezenbach,
	Carlo Caione, linux-clk
In-Reply-To: <20180827143200.8597-1-hdegoede@redhat.com>

On Mon, Aug 27, 2018 at 04:31:56PM +0200, Hans de Goede wrote:
> Hi All,
> 
> This series has as goal to revert commit d31fd43c0f9a ("clk: x86: Do not gate
> clocks enabled by the firmware"), because that commit causes almost all
> Cherry Trail devices to not use the S0i3 powerstate when suspending, leading
> to them quickly draining their battery when suspended.
> 
> This commit was added to fix issues with the r8169 NIC on some Bay Trail
> devices, where the pmc_plt_clk_4 was used as clk for the r8169 chip.
> 
> This series fixes this properly, so that we can undo the trouble some
> commit.
> 
> The first patch adds an "ether_clk" alias to the clk-pmc-atom driver so
> that the r8169 can pass "ether_clk" as generic id to clk_get instead of
> having to add x86 specific code to the r8169 driver.
> 
> The second patch makes the r8169 driver do a clk_get for "ether_clk"
> (ignoring -ENOENT errors so this is optional) and if that succeeds then
> it enables the clock.
> 
> The third patch effectively revert the troublesome commit.
> 
> This series has been tested on a HP Stream x360 - 11-p000nd, which uses
> a Bay Trail SoC with a r8169 ethernet chip and I can confirm that the
> pmc_plt_clk_4 gets properly enabled, so this series should not cause any
> regressions wrt devices needing pmc_plt_clk_4 enabled (which is not the
> case on the Stream x360).
> 
> To avoid regressions we need to have patches 1 and 2 merged before 3,
> so it is probably best if this entire series gets merged through a single
> tree. Given that clk-pmc-atom.c has not seen any changes for over a
> year I suggest that all 3 patches are merged through the netdev tree,
> with an ack from the clk maintainers. Assuming that is ok with the clk
> maintainers of course.
> 
> Note there is a fourth patch in this series, this patch is necessary to
> reach S0i3 state on Bay Trail devices wich have a r8169 ethernet chip,
> since I do not have access to hardware where the r8169 actually needs
> the pmc_plt_clk_4 I have not been able to test this, hence it is marked
> as RFC for now.
> 

What a nice stuff, thanks!

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Btw, you probably better to refer to
https://bugzilla.kernel.org/show_bug.cgi?id=196861 which is dedicated for S0ix
issue.

Another thing, clk_prepare_enable() can fail, I dunno what you can do at
->resume() stage with it failed.

> Carlos can you test the 4th patch (when you have time) and let us know if
> it works?
> 
> Regards,
> 
> Hans
> 

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATH v5 0/4] gpiolib: speed up GPIO array processing
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, Ulf Hansson, linux-doc, linux-iio, Dominik Brodowski,
	Peter Rosin, netdev, linux-i2c, Peter Meerwald-Stadler, devel,
	Florian Fainelli, Jonathan Corbet, Kishon Vijay Abraham I,
	Geert Uytterhoeven, linux-serial, Jiri Slaby, Michael Hennerich,
	linux-gpio, Lars-Peter Clausen, Greg Kroah-Hartman, linux-mmc,
	linux-kernel, Willy Tarreau, Miguel Ojeda Sandonis,
	Peter Korsgaard
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>


The goal is to boost performance of get/set array functions while
processing GPIO arrays which represent pins of a signle chip in
hardware order.  If resulting performance is close to PIO, GPIO API
can be used for data I/O without much loss of speed.

Created and tested on a low end Amstrad Delta board with NAND driver
updated to use GPIO API for data I/O.  Performance degrade compared to
PIO is much better than before the optimization but still not quite
satisfactory.

Janusz Krzysztofik (4):
      gpiolib: Pass bitmaps, not integer arrays, to get/set array
      gpiolib: Identify arrays matching GPIO hardware
      gpiolib: Pass array info to get/set array functions
      gpiolib: Implement fast processing path in get/set array

Changelog:
v5:
[PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set
- drivers/i2c/muxes/i2c-mux-gpio.c:
  - drop assigment of values to struct gpiomux.values, as recommended
    by Peter Rosin - thanks!,
  - mark the .values member of the structure as obsolete,
- drivers/mux/gpio.c:
  - drop assigment of values to struct mux_gpio.val, also recommended
    by Peter Rosin - thanks!,
  - merk the .val member of the structure as obsolete,
- drivers/auxdisplay/hd44780.c:
  - fix incorrect bitmap size,
  - use >>= operator to simplify notation,
  both catched by Miguel Ojeda - thanks!,
- add Cc: clauses as well as Acked-by: collected so far.
[PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
- add Cc: clause.
[PATCH v5 3/4] gpiolib: Pass array info to get/set array functions
- add Cc: clauses as well as Acked-by: collected so far.
[PATCH v5 4/4] gpiolib: Implement fast processing path in get/set
- add Cc: clause.

v4:
That series was a follow up of the former "mtd: rawnand: ams-delta: Use
gpio-omap accessors for data I/O" which already contained some changes
to gpiolib.  Those previous attempts were commented by Borris Brezillon
who suggested using GPIO API modified to accept bitmaps, and by Linus
Walleij who suggested still more great ideas for further immprovement
of the proposed API changes - thanks!

diffstat:
 Documentation/driver-api/gpio/board.rst     |   15 +
 Documentation/driver-api/gpio/consumer.rst  |   48 +++-
 drivers/auxdisplay/hd44780.c                |   64 +++---
 drivers/bus/ts-nbus.c                       |   25 --
 drivers/gpio/gpio-max3191x.c                |   23 +-
 drivers/gpio/gpiolib.c                      |  279 ++++++++++++++++++++++------
 drivers/gpio/gpiolib.h                      |   15 +
 drivers/i2c/muxes/i2c-mux-gpio.c            |   10 -
 drivers/mmc/core/pwrseq_simple.c            |   15 -
 drivers/mux/gpio.c                          |   12 -
 drivers/net/phy/mdio-mux-gpio.c             |    5 
 drivers/pcmcia/soc_common.c                 |   14 -
 drivers/phy/motorola/phy-mapphone-mdm6600.c |   21 +-
 drivers/staging/iio/adc/ad7606.c            |   12 -
 drivers/tty/serial/serial_mctrl_gpio.c      |    9 
 include/linux/gpio/consumer.h               |   35 ++-
 16 files changed, 412 insertions(+), 190 deletions(-)

^ permalink raw reply

* [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, Ulf Hansson, linux-doc, linux-iio, Dominik Brodowski,
	Peter Rosin, netdev, linux-i2c, Peter Meerwald-Stadler, devel,
	Florian Fainelli, Jonathan Corbet, Janusz Krzysztofik,
	Kishon Vijay Abraham I, Geert Uytterhoeven, linux-serial,
	Jiri Slaby, Michael Hennerich, linux-gpio, Lars-Peter Clausen,
	Greg Kroah-Hartman, linux-mmc, linux-kernel, Willy Tarreau,
	Miguel Ojeda Sandonis
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>

Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order.  If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.

While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip.  Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion.  Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 Documentation/driver-api/gpio/consumer.rst |  4 +-
 drivers/gpio/gpiolib.c                     | 72 +++++++++++++++++++++++++++++-
 drivers/gpio/gpiolib.h                     |  9 ++++
 include/linux/gpio/consumer.h              |  9 ++++
 4 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be obtained with one call::
 					   enum gpiod_flags flags)
 
 This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors.  It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
 
 	struct gpio_descs {
+		struct gpio_array *info;
 		unsigned int ndescs;
 		struct gpio_desc *desc[];
 	}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0e9ffa8cab6..c1ed1c759345 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 {
 	struct gpio_desc *desc;
 	struct gpio_descs *descs;
-	int count;
+	struct gpio_array *array_info = NULL;
+	struct gpio_chip *chip;
+	int count, bitmap_size;
 
 	count = gpiod_count(dev, con_id);
 	if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			gpiod_put_array(descs);
 			return ERR_CAST(desc);
 		}
+
 		descs->desc[descs->ndescs] = desc;
+
+		chip = gpiod_to_chip(desc);
+		/*
+		 * Select a chip of first array member
+		 * whose index matches its pin hardware number
+		 * as a candidate for fast bitmap processing.
+		 */
+		if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+			struct gpio_descs *array;
+
+			bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+						    chip->ngpio : count);
+
+			array = kzalloc(struct_size(descs, desc, count) +
+					struct_size(array_info, invert_mask,
+					3 * bitmap_size), GFP_KERNEL);
+			if (!array) {
+				gpiod_put_array(descs);
+				return ERR_PTR(-ENOMEM);
+			}
+
+			memcpy(array, descs,
+			       struct_size(descs, desc, descs->ndescs + 1));
+			kfree(descs);
+
+			descs = array;
+			array_info = (void *)(descs->desc + count);
+			array_info->get_mask = array_info->invert_mask +
+						  bitmap_size;
+			array_info->set_mask = array_info->get_mask +
+						  bitmap_size;
+
+			array_info->desc = descs->desc;
+			array_info->size = count;
+			array_info->chip = chip;
+			bitmap_set(array_info->get_mask, descs->ndescs,
+				   count - descs->ndescs);
+			bitmap_set(array_info->set_mask, descs->ndescs,
+				   count - descs->ndescs);
+			descs->info = array_info;
+		}
+		/*
+		 * Unmark members which don't qualify for fast bitmap
+		 * processing (different chip, not in hardware order)
+		 */
+		if (array_info && (chip != array_info->chip ||
+		    gpio_chip_hwgpio(desc) != descs->ndescs)) {
+			__clear_bit(descs->ndescs, array_info->get_mask);
+			__clear_bit(descs->ndescs, array_info->set_mask);
+		} else if (array_info) {
+			/* Exclude open drain or open source from fast output */
+			if (gpiochip_line_is_open_drain(chip, descs->ndescs) ||
+			    gpiochip_line_is_open_source(chip, descs->ndescs))
+				__clear_bit(descs->ndescs,
+					    array_info->set_mask);
+			/* Identify 'fast' pins which require invertion */
+			if (gpiod_is_active_low(desc))
+				__set_bit(descs->ndescs,
+					  array_info->invert_mask);
+		}
+
 		descs->ndescs++;
 	}
+	if (array_info)
+		dev_dbg(dev,
+			"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
+			array_info->chip->label, array_info->size,
+			*array_info->get_mask, *array_info->set_mask,
+			*array_info->invert_mask);
 	return descs;
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 11e83d2eef89..b60905d558b1 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -183,6 +183,15 @@ static inline bool acpi_can_fallback_to_crs(struct acpi_device *adev,
 }
 #endif
 
+struct gpio_array {
+	struct gpio_desc	**desc;
+	unsigned int		size;
+	struct gpio_chip	*chip;
+	unsigned long		*get_mask;
+	unsigned long		*set_mask;
+	unsigned long		invert_mask[];
+};
+
 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,
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 1b21dc7b0fad..8dede3e886af 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -17,11 +17,20 @@ struct device;
  */
 struct gpio_desc;
 
+/**
+ * Opaque descriptor for a structure of GPIO array attributes.  This structure
+ * is attached to struct gpiod_descs obtained from gpiod_get_array() and can be
+ * passed back to get/set array functions in order to activate fast processing
+ * path if applicable.
+ */
+struct gpio_array;
+
 /**
  * Struct containing an array of descriptors that can be obtained using
  * gpiod_get_array().
  */
 struct gpio_descs {
+	struct gpio_array *info;
 	unsigned int ndescs;
 	struct gpio_desc *desc[];
 };
-- 
2.16.4

^ permalink raw reply related

* [PATCH v5 3/4] gpiolib: Pass array info to get/set array functions
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>

In order to make use of array info obtained from gpiod_get_array() and
speed up processing of arrays matching single GPIO chip layout, that
information must be passed to get/set array functions.  Extend the
functions' API with that additional parameter and update all users.
Pass NULL if a user bulids an array itself from single GPIOs.

Cc: Jonathan Corbet <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  | 14 ++++++++++--
 drivers/auxdisplay/hd44780.c                | 12 ++++++----
 drivers/bus/ts-nbus.c                       |  6 +++--
 drivers/gpio/gpio-max3191x.c                |  6 +++--
 drivers/gpio/gpiolib.c                      | 34 ++++++++++++++++++++---------
 drivers/gpio/gpiolib.h                      |  2 ++
 drivers/i2c/muxes/i2c-mux-gpio.c            |  2 +-
 drivers/mmc/core/pwrseq_simple.c            |  2 +-
 drivers/mux/gpio.c                          |  3 ++-
 drivers/net/phy/mdio-mux-gpio.c             |  2 +-
 drivers/pcmcia/soc_common.c                 |  3 ++-
 drivers/phy/motorola/phy-mapphone-mdm6600.c |  4 +++-
 drivers/staging/iio/adc/ad7606.c            |  3 ++-
 drivers/tty/serial/serial_mctrl_gpio.c      |  2 +-
 include/linux/gpio/consumer.h               |  8 +++++++
 15 files changed, 75 insertions(+), 28 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index 7e0298b9a7b9..0afd95a12b10 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs::
 
 	int gpiod_get_array_value(unsigned int array_size,
 				  struct gpio_desc **desc_array,
+				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap);
 	int gpiod_get_raw_array_value(unsigned int array_size,
 				      struct gpio_desc **desc_array,
+				      struct gpio_array *array_info,
 				      unsigned long *value_bitmap);
 	int gpiod_get_array_value_cansleep(unsigned int array_size,
 					   struct gpio_desc **desc_array,
+					   struct gpio_array *array_info,
 					   unsigned long *value_bitmap);
 	int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 					   struct gpio_desc **desc_array,
+					   struct gpio_array *array_info,
 					   unsigned long *value_bitmap);
 
 	void gpiod_set_array_value(unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap)
 	void gpiod_set_raw_array_value(unsigned int array_size,
 				       struct gpio_desc **desc_array,
+				       struct gpio_array *array_info,
 				       unsigned long *value_bitmap)
 	void gpiod_set_array_value_cansleep(unsigned int array_size,
 					    struct gpio_desc **desc_array,
+					    struct gpio_array *array_info,
 					    unsigned long *value_bitmap)
 	void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 						struct gpio_desc **desc_array,
+						struct gpio_array *array_info,
 						unsigned long *value_bitmap)
 
 The array can be an arbitrary set of GPIOs. The functions will try to access
@@ -358,6 +366,7 @@ accessed sequentially.
 The functions take three arguments:
 	* array_size	- the number of array elements
 	* desc_array	- an array of GPIO descriptors
+	* array_info	- optional information obtained from gpiod_array_get()
 	* value_bitmap	- a bitmap to store the GPIOs' values (get) or
 			  a bitmap of values to assign to the GPIOs (set)
 
@@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array()::
 
 	struct gpio_descs *my_gpio_descs = gpiod_get_array(...);
 	gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc,
-			      my_gpio_value_bitmap);
+			      my_gpio_descs->info, my_gpio_value_bitmap);
 
 It is also possible to access a completely arbitrary array of descriptors. The
 descriptors may be obtained using any combination of gpiod_get() and
 gpiod_get_array(). Afterwards the array of descriptors has to be setup
-manually before it can be passed to one of the above functions.
+manually before it can be passed to one of the above functions.  In that case,
+array_info should be set to NULL.
 
 Note that for optimal performance GPIOs belonging to the same chip should be
 contiguous within the array of descriptors.
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index bbbd6a29bf01..ec20c41831b0 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -74,7 +74,8 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
 	}
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 }
@@ -97,7 +98,8 @@ static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
 	value_bitmap[0] >>= PIN_DATA4;
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 
@@ -106,7 +108,8 @@ static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
 	value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 }
@@ -169,7 +172,8 @@ static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
 	value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 }
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index ce6c1e89236d..000d756eb42c 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -112,7 +112,8 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
 {
 	unsigned long value_bitmap[1] = { 0, };
 
-	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, value_bitmap);
+	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc,
+				       ts_nbus->data->info, 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);
@@ -155,7 +156,8 @@ static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
 	struct gpio_descs *gpios = ts_nbus->data;
 	unsigned long value_bitmap[1] = { byte, };
 
-	gpiod_set_array_value_cansleep(8, gpios->desc, value_bitmap);
+	gpiod_set_array_value_cansleep(8, gpios->desc, gpios->info,
+				       value_bitmap);
 }
 
 /*
diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index c4ec1c82af27..4b43b5dabfd2 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -313,6 +313,7 @@ static int max3191x_set_config(struct gpio_chip *gpio, unsigned int offset,
 
 static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
 						  struct gpio_desc **desc,
+						  struct gpio_array *info,
 						  int value)
 {
 	unsigned long *value_bitmap;
@@ -327,7 +328,7 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
 	else
 		bitmap_zero(value_bitmap, ndescs);
 
-	gpiod_set_array_value_cansleep(ndescs, desc, value_bitmap);
+	gpiod_set_array_value_cansleep(ndescs, desc, info, value_bitmap);
 	kfree(value_bitmap);
 }
 
@@ -400,7 +401,8 @@ static int max3191x_probe(struct spi_device *spi)
 	if (max3191x->modesel_pins)
 		gpiod_set_array_single_value_cansleep(
 				 max3191x->modesel_pins->ndescs,
-				 max3191x->modesel_pins->desc, max3191x->mode);
+				 max3191x->modesel_pins->desc,
+				 max3191x->modesel_pins->info, max3191x->mode);
 
 	max3191x->ignore_uv = device_property_read_bool(dev,
 						  "maxim,ignore-undervoltage");
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c1ed1c759345..4d26cdbdb7cf 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -435,7 +435,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 		int ret = gpiod_get_array_value_complex(false,
 							true,
 							lh->numdescs,
-							lh->descs,
+							lh->descs, NULL,
 							value_bitmap);
 		if (ret)
 			return ret;
@@ -467,7 +467,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 		return gpiod_set_array_value_complex(false,
 					      true,
 					      lh->numdescs,
-					      lh->descs,
+					      lh->descs, NULL,
 					      value_bitmap);
 	}
 	return -EINVAL;
@@ -2784,6 +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,
+				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap)
 {
 	int i = 0;
@@ -2908,12 +2909,14 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
  */
 int gpiod_get_raw_array_value(unsigned int array_size,
 			      struct gpio_desc **desc_array,
+			      struct gpio_array *array_info,
 			      unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(true, false, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
 
@@ -2931,12 +2934,14 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
  */
 int gpiod_get_array_value(unsigned int array_size,
 			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
 			  unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(false, false, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array_value);
 
@@ -3029,6 +3034,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,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap)
 {
 	int i = 0;
@@ -3166,12 +3172,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
  */
 int gpiod_set_raw_array_value(unsigned int array_size,
 			 struct gpio_desc **desc_array,
+			 struct gpio_array *array_info,
 			 unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_set_array_value_complex(true, false, array_size,
-					desc_array, value_bitmap);
+					desc_array, array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
 
@@ -3189,12 +3196,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
  */
 void gpiod_set_array_value(unsigned int array_size,
 			   struct gpio_desc **desc_array,
+			   struct gpio_array *array_info,
 			   unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return;
 	gpiod_set_array_value_complex(false, false, array_size, desc_array,
-				      value_bitmap);
+				      array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value);
 
@@ -3426,13 +3434,15 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
  */
 int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 				       struct gpio_desc **desc_array,
+				       struct gpio_array *array_info,
 				       unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(true, true, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
 
@@ -3449,13 +3459,15 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
  */
 int gpiod_get_array_value_cansleep(unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   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_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
 
@@ -3508,13 +3520,14 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
  */
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 					struct gpio_desc **desc_array,
+					struct gpio_array *array_info,
 					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_bitmap);
+				      array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
 
@@ -3548,13 +3561,14 @@ 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,
+				    struct gpio_array *array_info,
 				    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_bitmap);
+				      array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index b60905d558b1..b65ca896b24d 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -196,10 +196,12 @@ 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,
+				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap);
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				   unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap);
 
 /* This is just passed between gpiolib and devres */
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 4e36e0eac7a3..4439a92c86a2 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -31,7 +31,7 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
 	int i;
 
 	gpiod_set_array_value_cansleep(mux->data.n_gpios,
-				       mux->gpios, value_bitmap);
+				       mux->gpios, NULL, 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 0d6e3a5be3ba..5cf7eda8f68f 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -46,7 +46,7 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 		value_bitmap[0] = value;
 
 		gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
-					       value_bitmap);
+					       reset_gpios->info, value_bitmap);
 	}
 }
 
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index 734e1b43aed6..be8c86680e10 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -27,7 +27,8 @@ static int mux_gpio_set(struct mux_control *mux, int state)
 	int i;
 
 	gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
-				       mux_gpio->gpios->desc, value_bitmap);
+				       mux_gpio->gpios->desc,
+				       mux_gpio->gpios->info, value_bitmap);
 
 	return 0;
 }
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 8e1ec750277e..c0ffa03c916b 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -37,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,
-				       value_bitmap);
+				       s->gpios->info, value_bitmap);
 
 	return 0;
 }
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index e0f89155c474..55978198cd2b 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -366,7 +366,8 @@ static int soc_common_pcmcia_config_skt(
 		}
 
 		if (n)
-			gpiod_set_array_value_cansleep(n, descs, value_bitmap);
+			gpiod_set_array_value_cansleep(n, descs, NULL,
+						       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 b6477c3599c4..8f508338ec56 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -162,7 +162,8 @@ static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
 	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, value_bitmap);
+				       ddata->cmd_gpios->desc,
+				       ddata->cmd_gpios->info, value_bitmap);
 }
 
 /**
@@ -181,6 +182,7 @@ static void phy_mdm6600_status(struct work_struct *work)
 
 	error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
 					       ddata->status_gpios->desc,
+					       ddata->status_gpios->info,
 					       value_bitmap);
 	if (error)
 		return;
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 0eca047bc1cc..eb779d825724 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -230,7 +230,8 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 		value_bitmap[0] = ret;
 
 		mutex_lock(&st->lock);
-		gpiod_set_array_value(3, st->gpio_os->desc, value_bitmap);
+		gpiod_set_array_value(3, st->gpio_os->desc, st->gpio_os->info,
+				      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 bb8b4756d72d..8a04e3be5419 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -53,7 +53,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
 				     !!(mctrl & mctrl_gpios_desc[i].mctrl));
 			count++;
 		}
-	gpiod_set_array_value(count, desc_array, value_bitmap);
+	gpiod_set_array_value(count, desc_array, NULL, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_set);
 
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 8dede3e886af..bf037ebe2ed8 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -114,36 +114,44 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
 int gpiod_get_value(const struct gpio_desc *desc);
 int gpiod_get_array_value(unsigned int array_size,
 			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
 			  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,
+			   struct gpio_array *array_info,
 			   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,
+			      struct gpio_array *array_info,
 			      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,
+			       struct gpio_array *array_info,
 			       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,
+				   struct gpio_array *array_info,
 				   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,
+				    struct gpio_array *array_info,
 				    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,
+				       struct gpio_array *array_info,
 				       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,
+					struct gpio_array *array_info,
 					unsigned long *value_bitmap);
 
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
-- 
2.16.4

^ permalink raw reply related

* [PATCH v5 4/4] gpiolib: Implement fast processing path in get/set array
From: Janusz Krzysztofik @ 2018-08-29 20:49 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>

Certain GPIO descriptor arrays returned by gpio_get_array() may contain
information on direct mapping of array members to pins of a single GPIO
chip in hardware order.  In such cases, bitmaps of values can be passed
directly from/to the chip's .get/set_multiple() callbacks without
wasting time on iterations.

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

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 Documentation/driver-api/gpio/board.rst    | 15 ++++++
 Documentation/driver-api/gpio/consumer.rst |  8 +++
 drivers/gpio/gpiolib.c                     | 87 ++++++++++++++++++++++++++++--
 3 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/Documentation/driver-api/gpio/board.rst b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
 
 The line will be hogged as soon as the gpiochip is created or - in case the
 chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--------------
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function.  The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing.  If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
 Note that for optimal performance GPIOs belonging to the same chip should be
 contiguous within the array of descriptors.
 
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip.  If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip.  That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
 The return value of gpiod_get_array_value() and its variants is 0 on success
 or negative on error. Note the difference to gpiod_get_value(), which returns
 0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d26cdbdb7cf..b799a89c4c17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2787,7 +2787,36 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap)
 {
-	int i = 0;
+	int err, i = 0;
+
+	/*
+	 * Validate array_info against desc_array and its size.
+	 * It should immediately follow desc_array if both
+	 * have been obtained from the same gpiod_get_array() call.
+	 */
+	if (array_info && array_info->desc == desc_array &&
+	    array_size <= array_info->size &&
+	    (void *)array_info == desc_array + array_info->size) {
+		if (!can_sleep)
+			WARN_ON(array_info->chip->can_sleep);
+
+		err = gpio_chip_get_multiple(array_info->chip,
+					     array_info->get_mask,
+					     value_bitmap);
+		if (err)
+			return err;
+
+		if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+			bitmap_xor(value_bitmap, value_bitmap,
+				   array_info->invert_mask, array_size);
+
+		if (bitmap_full(array_info->get_mask, array_size))
+			return 0;
+
+		i = find_first_zero_bit(array_info->get_mask, array_size);
+	} else {
+		array_info = NULL;
+	}
 
 	while (i < array_size) {
 		struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2818,7 +2847,12 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 			int hwgpio = gpio_chip_hwgpio(desc);
 
 			__set_bit(hwgpio, mask);
-			i++;
+
+			if (array_info)
+				find_next_zero_bit(array_info->get_mask,
+						   array_size, i);
+			else
+				i++;
 		} while ((i < array_size) &&
 			 (desc_array[i]->gdev->chip == chip));
 
@@ -2829,7 +2863,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 			return ret;
 		}
 
-		for (j = first; j < i; j++) {
+		for (j = first; j < i; ) {
 			const struct gpio_desc *desc = desc_array[j];
 			int hwgpio = gpio_chip_hwgpio(desc);
 			int value = test_bit(hwgpio, bits);
@@ -2838,6 +2872,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				value = !value;
 			__assign_bit(j, value_bitmap, value);
 			trace_gpio_value(desc_to_gpio(desc), 1, value);
+
+			if (array_info)
+				find_next_zero_bit(array_info->get_mask, i, j);
+			else
+				j++;
 		}
 
 		if (mask != fastpath)
@@ -3039,6 +3078,32 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 {
 	int i = 0;
 
+	/*
+	 * Validate array_info against desc_array and its size.
+	 * It should immediately follow desc_array if both
+	 * have been obtained from the same gpiod_get_array() call.
+	 */
+	if (array_info && array_info->desc == desc_array &&
+	    array_size <= array_info->size &&
+	    (void *)array_info == desc_array + array_info->size) {
+		if (!can_sleep)
+			WARN_ON(array_info->chip->can_sleep);
+
+		if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+			bitmap_xor(value_bitmap, value_bitmap,
+				   array_info->invert_mask, array_size);
+
+		gpio_chip_set_multiple(array_info->chip, array_info->set_mask,
+				       value_bitmap);
+
+		if (bitmap_full(array_info->set_mask, array_size))
+			return 0;
+
+		i = find_first_zero_bit(array_info->set_mask, array_size);
+	} else {
+		array_info = NULL;
+	}
+
 	while (i < array_size) {
 		struct gpio_chip *chip = desc_array[i]->gdev->chip;
 		unsigned long fastpath[2 * BITS_TO_LONGS(FASTPATH_NGPIO)];
@@ -3066,7 +3131,14 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 			int hwgpio = gpio_chip_hwgpio(desc);
 			int value = test_bit(i, value_bitmap);
 
-			if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+			/*
+			 * Pins applicable for fast input but not for
+			 * fast output processing may have been already
+			 * inverted inside the fast path, skip them.
+			 */
+			if (!raw && !(array_info &&
+			    test_bit(i, array_info->invert_mask)) &&
+			    test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 				value = !value;
 			trace_gpio_value(desc_to_gpio(desc), 0, value);
 			/*
@@ -3085,7 +3157,12 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 					__clear_bit(hwgpio, bits);
 				count++;
 			}
-			i++;
+
+			if (array_info)
+				find_next_zero_bit(array_info->set_mask,
+						   array_size, i);
+			else
+				i++;
 		} while ((i < array_size) &&
 			 (desc_array[i]->gdev->chip == chip));
 		/* push collected bits to outputs */
-- 
2.16.4

^ permalink raw reply related

* Re: [pull request][net-next 00/10] Mellanox, mlx5 and devlink updates 2018-07-31
From: Alexander Duyck @ 2018-08-29 17:04 UTC (permalink / raw)
  To: valex
  Cc: Saeed Mahameed, Saeed Mahameed, David Miller, Netdev, Jiri Pirko,
	Jakub Kicinski, Bjorn Helgaas
In-Reply-To: <b8b1444f-ee31-b6c1-a5e2-aa0d2e08d2e5@mellanox.com>

On Wed, Aug 29, 2018 at 8:43 AM Alex Vesker <valex@mellanox.com> wrote:
>
>
> > On Wed, Aug 1, 2018 at 4:13 PM, Saeed Mahameed
> > <saeedm@dev.mellanox.co.il> wrote:
> >> On Wed, Aug 1, 2018 at 3:34 PM, Alexander Duyck
> >> <alexander.duyck@gmail.com> wrote:
> >>> On Wed, Aug 1, 2018 at 2:52 PM, Saeed Mahameed <saeedm@mellanox.com>
> >>> wrote:
> >>>> Hi Dave,
> >>>>
> >>>> This series provides devlink parameters updates to both devlink API
> >>>> and
> >>>> mlx5 driver, it is a 2nd iteration of the dropped patches sent in a
> >>>> previous
> >>>> mlx5 submission "net/mlx5: Support PCIe buffer congestion handling via
> >>>> Devlink" to address review comments [1].
> >>>>
> >>>> Changes from the original series:
> >>>> - According to the discussion outcome, we are keeping the
> >>>> congestion control
> >>>>   setting as mlx5 device specific for the current HW generation.
> >>>> - Changed the congestion_mode and congestion action param type to
> >>>> string
> >>>> - Added patches to fix devlink handling of param type string
> >>>> - Added a patch which adds extack messages support for param set.
> >>>> - At the end of this series, I've added yet another mlx5 devlink
> >>>> related
> >>>>  feature, firmware snapshot support.
> >>>>
> >>>> For more information please see tag log below.
> >>>>
> >>>> Please pull and let me know if there's any problem.
> >>>>
> >>>> [1] https://patchwork.ozlabs.org/patch/945996/
> >>>>
> >>>> Thanks,
> >>>> Saeed.
> >>>>
> >>>> ---
> >>>>
> >>>> The following changes since commit
> >>>> e6476c21447c4b17c47e476aade6facf050f31e8:
> >>>>
> >>>>   net: remove bogus RCU annotations on socket.wq (2018-07-31
> >>>> 12:40:22 -0700)
> >>>>
> >>>> are available in the Git repository at:
> >>>>
> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git
> >>>> tags/mlx5-updates-2018-08-01
> >>>>
> >>>> for you to fetch changes up to
> >>>> 2ac6108c65ffcb1e5eab1fba1fd59272604d1c32:
> >>>>
> >>>>   net/mlx5: Use devlink region_snapshot parameter (2018-08-01
> >>>> 14:49:09 -0700)
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> mlx5-updates-2018-08-01
> >>>>
> >>>> This series provides devlink parameters updates to both devlink API
> >>>> and
> >>>> mlx5 driver,
> >>>>
> >>>> 1) Devlink changes: (Moshe Shemesh)
> >>>> The first two patches fix devlink param infrastructure for string type
> >>>> params.
> >>>> The third patch adds a devlink helper function to safely copy
> >>>> string from
> >>>> driver to devlink.
> >>>> The forth patch adds extack support for param set.
> >>>>
> >>>> 2) mlx5 specific congestion parameters: (Eran Ben Elisha)
> >>>> Next three patches add new devlink driver specific params for
> >>>> controlling
> >>>> congestion action and mode, using string type params and extack
> >>>> messages support.
> >>>>
> >>>> This congestion mode enables hw workaround in specific devices
> >>>> which is
> >>>> controlled by devlink driver-specific params. The workaround is device
> >>>> specific for this NIC generation, the next NIC will not need it.
> >>>>
> >>>> Congestion parameters:
> >>>>  - Congestion action
> >>>>             HW W/A mechanism in the PCIe buffer which monitors the
> >>>> amount of
> >>>>             consumed PCIe buffer per host.  This mechanism supports
> >>>> the
> >>>>             following actions in case of threshold overflow:
> >>>>             - Disabled - NOP (Default)
> >>>>             - Drop
> >>>>             - Mark - Mark CE bit in the CQE of received packet
> >>>>     - Congestion mode
> >>>>             - Aggressive - Aggressive static trigger threshold
> >>>> (Default)
> >>>>             - Dynamic - Dynamically change the trigger threshold
> >>>>
> >>>> 3) mlx5 firmware snapshot support via devlink: (Alex Vesker)
> >>>> Last three patches, add the support for capturing region snapshot
> >>>> of the
> >>>> firmware crspace during critical errors, using devlink region_snapshot
> >>>> parameter.
> >>>>
> >>>> -Saeed.
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> Alex Vesker (3):
> >>>>       net/mlx5: Add Vendor Specific Capability access gateway
> >>>>       net/mlx5: Add Crdump FW snapshot support
> >>>>       net/mlx5: Use devlink region_snapshot parameter
> >>>>
> >>>> Eran Ben Elisha (3):
> >>>>       net/mlx5: Move all devlink related functions calls to devlink.c
> >>>>       net/mlx5: Add MPEGC register configuration functionality
> >>>>       net/mlx5: Enable PCIe buffer congestion handling workaround
> >>>> via devlink
> >>>>
> >>>> Moshe Shemesh (4):
> >>>>       devlink: Fix param set handling for string type
> >>>>       devlink: Fix param cmode driverinit for string type
> >>>>       devlink: Add helper function for safely copy string param
> >>>>       devlink: Add extack messages support to param set
> >>>>
> >>>>  drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c  |   3 +-
> >>>>  drivers/net/ethernet/mellanox/mlx4/main.c          |   6 +-
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   3 +-
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/devlink.c  | 388
> >>>> +++++++++++++++++++++
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/devlink.h  |  13 +
> >>>>  .../net/ethernet/mellanox/mlx5/core/diag/crdump.c  | 223 ++++++++++++
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/health.c   |   3 +
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/lib/mlx5.h |   4 +
> >>>>  .../net/ethernet/mellanox/mlx5/core/lib/pci_vsc.c  | 320
> >>>> +++++++++++++++++
> >>>>  .../net/ethernet/mellanox/mlx5/core/lib/pci_vsc.h  |  56 +++
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/main.c     |  10 +-
> >>>>  include/linux/mlx5/driver.h                        |   5 +
> >>>>  include/net/devlink.h                              |  15 +-
> >>>>  net/core/devlink.c                                 |  44 ++-
> >>>>  14 files changed, 1076 insertions(+), 17 deletions(-)
> >>>>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> >>>>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/devlink.h
> >>>>  create mode 100644
> >>>> drivers/net/ethernet/mellanox/mlx5/core/diag/crdump.c
> >>>>  create mode 100644
> >>>> drivers/net/ethernet/mellanox/mlx5/core/lib/pci_vsc.c
> >>>>  create mode 100644
> >>>> drivers/net/ethernet/mellanox/mlx5/core/lib/pci_vsc.h
> >>>
> >>> So after looking over the patch set the one thing I would ask for in
> >>> this is some sort of documentation at a minimum. As a user I don't see
> >>> how you can expect someone to be able to use this when the naming of
> >>> things are pretty cryptic and there is no real explanation anywhere if
> >>> you don't go through and read the patch description itself. When you
> >>> start adding driver specific interfaces, you should at least start
> >>> adding vendor specific documentation.
> >>>
> >>
> >> Sure, sounds like a great idea, something like:
> >> Documentation/networking/mlx5.txt and have a devlink section ?
> >> or have a generic devlink doc and a mlx5 section in it ?
> >
> > Either would work for me.
> >
> For which patches are you missing documentation?

Any interfaces that you are exposing via the devlink interface should
have a document somewhere that explains if, when, and how to use it.
All of these new interfaces you were adding in the patch set has no
explanation of what they are other than what is in the driver code
itself and most users don't want to try and decode the driver itself
to figure out what it is they need to do to use an interface.

> >>> Also I don't see how using a vendor specific configuration space
> >>> section can be done without adding some tie-ins to the PCI core files
> >>> because it should be possible to race with someone poking at the
> >>> register space via something like setpci/lspci. Also one of the things
> >>> that came up was that drivers are not supposed to be banging on the
> >>> PCI configuration space at will, and it seems like this patch set is
> >>> doing exactly that through the VSC block.
> >>>
> >>
> >> this is a whole different feature than the device specific parameters.
> >> The whole vendor specific configuration space access is needed only
> >> for diagnostic/dump
> >> purposes when something really bad happens and the command interface
> >> with FW is down,
> >> and when the FW is un-responsive, we want to dump the crspace into the
> >> already existing devlink
> >> crdump buffer, how do you expect us to read it if we are not allowed
> >> to access it ?
> >>
> >> What do you mean by tie-ins to the PCI core files ? can you please
> >> elaborate ?
> >
> > You have added a vendor specific config section and you are using it
> > to access several of the pieces of metadata. The setup isn't too
> > different than the VPD setup and approach. However I don't see many of
> > the protections that exist for VPD in place for this vendor specific
> > configuration. As such I have concerns. For example what is to keep
> > requests to the various devlink interfaces from racing with each other
> > when they both end up operating through the VCS?
> >
> > - Alex
>
> Hi, I would like to resubmit the devlink region crdump support for mlx5,
> which is part of this patch-set.
>
> AlexD, regarding the protection, various devlink interfaces cannot race
> since
> devlink_mutex is used. The VSC access is also protected, using
> mlx5_vsc_gw_lock/unlock
> only one can acquire the lock.

You are talking about from within the driver right? What about raw PCI
configuration space access? Is there anything to keep setpci from
causing issues when you are using the interface? Really creating a
vendor specific config block in and of itself creates a whole new
mess.

> After explaining this I want to clarify something, the access to VSC is
> not user driven
> it happens automatically by the driver when an error is detected to
> collect a crdump.

You don't prevent a user from being able to poke at the configuration
space via setpci.

^ permalink raw reply

* Re: [PATCH 0/4] clk-pmc-atom + r8169: Add ether_clk handling to fix suspend issues
From: Hans de Goede @ 2018-08-29 17:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David S . Miller, Heiner Kallweit, Michael Turquette,
	Stephen Boyd, Irina Tirdea, netdev, Johannes Stezenbach,
	Carlo Caione, linux-clk
In-Reply-To: <20180829163141.GB8462@smile.fi.intel.com>

Hi,

On 29-08-18 18:31, Andy Shevchenko wrote:
> On Mon, Aug 27, 2018 at 04:31:56PM +0200, Hans de Goede wrote:
>> Hi All,
>>
>> This series has as goal to revert commit d31fd43c0f9a ("clk: x86: Do not gate
>> clocks enabled by the firmware"), because that commit causes almost all
>> Cherry Trail devices to not use the S0i3 powerstate when suspending, leading
>> to them quickly draining their battery when suspended.
>>
>> This commit was added to fix issues with the r8169 NIC on some Bay Trail
>> devices, where the pmc_plt_clk_4 was used as clk for the r8169 chip.
>>
>> This series fixes this properly, so that we can undo the trouble some
>> commit.
>>
>> The first patch adds an "ether_clk" alias to the clk-pmc-atom driver so
>> that the r8169 can pass "ether_clk" as generic id to clk_get instead of
>> having to add x86 specific code to the r8169 driver.
>>
>> The second patch makes the r8169 driver do a clk_get for "ether_clk"
>> (ignoring -ENOENT errors so this is optional) and if that succeeds then
>> it enables the clock.
>>
>> The third patch effectively revert the troublesome commit.
>>
>> This series has been tested on a HP Stream x360 - 11-p000nd, which uses
>> a Bay Trail SoC with a r8169 ethernet chip and I can confirm that the
>> pmc_plt_clk_4 gets properly enabled, so this series should not cause any
>> regressions wrt devices needing pmc_plt_clk_4 enabled (which is not the
>> case on the Stream x360).
>>
>> To avoid regressions we need to have patches 1 and 2 merged before 3,
>> so it is probably best if this entire series gets merged through a single
>> tree. Given that clk-pmc-atom.c has not seen any changes for over a
>> year I suggest that all 3 patches are merged through the netdev tree,
>> with an ack from the clk maintainers. Assuming that is ok with the clk
>> maintainers of course.
>>
>> Note there is a fourth patch in this series, this patch is necessary to
>> reach S0i3 state on Bay Trail devices wich have a r8169 ethernet chip,
>> since I do not have access to hardware where the r8169 actually needs
>> the pmc_plt_clk_4 I have not been able to test this, hence it is marked
>> as RFC for now.
>>
> 
> What a nice stuff, thanks!
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thank you (and also thank you for the other reviews)

> Btw, you probably better to refer to
> https://bugzilla.kernel.org/show_bug.cgi?id=196861 which is dedicated for S0ix
> issue.

Ok, I've added a link to that. I've also added:

Reported-by: Johannes Stezenbach <js@sig21.net>

To honor Johannes for figuring out that leaving the clocks enabled
was a problem in the first place.

This will all be included in v2 of the series when I send it out.

> Another thing, clk_prepare_enable() can fail, I dunno what you can do at
> ->resume() stage with it failed.

Right, I know that it can fail, but in practice it never fails unless
things are seriously foo-barred already and there is not much we can
do when it fails, so I decided to just ignore the return code.

Regards,

Hans

^ permalink raw reply

* [Patch iproute2 v2] ss: add UNIX_DIAG_VFS and UNIX_DIAG_ICONS for unix sockets
From: Cong Wang @ 2018-08-29 17:09 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Stephen Hemminger

UNIX_DIAG_VFS and UNIX_DIAG_ICONS are never used by ss,
make them available in ss -e output.

Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 misc/ss.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/misc/ss.c b/misc/ss.c
index 41e7762b..b2c634c8 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -16,6 +16,7 @@
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 #include <sys/uio.h>
+#include <sys/sysmacros.h>
 #include <netinet/in.h>
 #include <string.h>
 #include <errno.h>
@@ -3604,6 +3605,21 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			out(" %c-%c",
 			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
+		if (tb[UNIX_DIAG_VFS]) {
+			struct unix_diag_vfs *uv = RTA_DATA(tb[UNIX_DIAG_VFS]);
+
+			out(" ino:%u dev:%u/%u", uv->udiag_vfs_ino, major(uv->udiag_vfs_dev),
+						 minor(uv->udiag_vfs_dev));
+		}
+		if (tb[UNIX_DIAG_ICONS]) {
+			int len = RTA_PAYLOAD(tb[UNIX_DIAG_ICONS]);
+			__u32 *peers = RTA_DATA(tb[UNIX_DIAG_ICONS]);
+			int i;
+
+			out(" peers:");
+			for (i = 0; i < len / sizeof(__u32); i++)
+				out(" %u", peers[i]);
+		}
 	}
 
 	return 0;
@@ -3641,6 +3657,8 @@ static int unix_show_netlink(struct filter *f)
 	req.r.udiag_show = UDIAG_SHOW_NAME | UDIAG_SHOW_PEER | UDIAG_SHOW_RQLEN;
 	if (show_mem)
 		req.r.udiag_show |= UDIAG_SHOW_MEMINFO;
+	if (show_details)
+		req.r.udiag_show |= UDIAG_SHOW_VFS | UDIAG_SHOW_ICONS;
 
 	return handle_netlink_request(f, &req.nlh, sizeof(req), unix_show_sock);
 }
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
From: Hans de Goede @ 2018-08-29 17:09 UTC (permalink / raw)
  To: Stephen Boyd, David S . Miller, Andy Shevchenko, Heiner Kallweit,
	Irina Tirdea, Michael Turquette
  Cc: netdev, Johannes Stezenbach, Carlo Caione, linux-clk
In-Reply-To: <153539729389.129321.804899531283282568@swboyd.mtv.corp.google.com>

Hi,

On 27-08-18 21:14, Stephen Boyd wrote:
> Quoting Hans de Goede (2018-08-27 11:53:19)
>> On 27-08-18 20:47, Stephen Boyd wrote:
>>> How would you know that a clk device driver hasn't probed yet and isn't
>>> the driver that's actually providing the clk to this device on x86
>>> systems? With DT systems we can figure that out by looking at the DT and
>>> seeing if the device driver requesting the clk has the clocks property.
>>> On x86 systems it's all clkdev which doesn't really lend itself to
>>> solving this problem.
>>
>> Right on x86 the assumption is that the clk driver will be builtin and
>> will probe before the consumer. In this case that is true as the
>> pmc-atom-clk driver can only be builtin and its platform device is
>> instantiated from the acpi_lpss code and acpi init happens before
>> the PCI bus is scanned.
> 
> If we can go with this assumption then we can make the optional clk API
> work even on clkdev based systems. Maybe if x86 had some way of
> indicating that all builtin clks are registered?

Unfortunately there is no such thing I'm afraid.

> That might work but
> it's not very clean. Or if we could check to see if we're running on an
> ACPI based system in clkdev we could use that to assume that clk_get()
> will only be called after all providers have registered their lookups.

Yes some check for x86 + ACPI (ARM also uses ACPI, but there we
should no do this AFAICT) is probably best. That or not use the
new optional clk API on x86, but that means that any cross platform
driver cannot use it, which would be a pain.

BTW does your Acked-by indicate you are ok with merging this series
through the netdev tree as I suggested in the cover-letter? If so
can I also add your Acked-by to the 3th patch ?

Regards,

Hans

^ permalink raw reply

* [Patch net-nnext] net_sched: add missing tcf_lock for act_connmark
From: Cong Wang @ 2018-08-29 17:15 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Vlad Buslov
In-Reply-To: <20180829171536.23162-1-xiyou.wangcong@gmail.com>

According to the new locking rule, we have to take tcf_lock
for both ->init() and ->dump(), as RTNL will be removed.
However, it is missing for act_connmark.

Cc: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_connmark.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index e869c0ee63c8..8475913f2070 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -143,8 +143,10 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
 			return -EEXIST;
 		}
 		/* replacing action and zone */
+		spin_lock_bh(&ci->tcf_lock);
 		ci->tcf_action = parm->action;
 		ci->zone = parm->zone;
+		spin_unlock_bh(&ci->tcf_lock);
 		ret = 0;
 	}
 
@@ -156,16 +158,16 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_connmark_info *ci = to_connmark(a);
-
 	struct tc_connmark opt = {
 		.index   = ci->tcf_index,
 		.refcnt  = refcount_read(&ci->tcf_refcnt) - ref,
 		.bindcnt = atomic_read(&ci->tcf_bindcnt) - bind,
-		.action  = ci->tcf_action,
-		.zone   = ci->zone,
 	};
 	struct tcf_t t;
 
+	spin_lock_bh(&ci->tcf_lock);
+	opt.action = ci->tcf_action;
+	opt.zone = ci->zone;
 	if (nla_put(skb, TCA_CONNMARK_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
@@ -173,9 +175,12 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
 	if (nla_put_64bit(skb, TCA_CONNMARK_TM, sizeof(t), &t,
 			  TCA_CONNMARK_PAD))
 		goto nla_put_failure;
+	spin_unlock_bh(&ci->tcf_lock);
 
 	return skb->len;
+
 nla_put_failure:
+	spin_unlock_bh(&ci->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
-- 
2.14.4

^ permalink raw reply related

* [Patch net-nnext] Revert "net: sched: act: add extack for lookup callback"
From: Cong Wang @ 2018-08-29 17:15 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Alexander Aring

This reverts commit 331a9295de23 ("net: sched: act: add extack for lookup callback").

This extack is never used after 6 months... In fact, it can be just
set in the caller, right after ->lookup().

Cc: Alexander Aring <aring@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/act_api.h      | 3 +--
 net/sched/act_api.c        | 6 ++++--
 net/sched/act_bpf.c        | 3 +--
 net/sched/act_connmark.c   | 3 +--
 net/sched/act_csum.c       | 3 +--
 net/sched/act_gact.c       | 3 +--
 net/sched/act_ife.c        | 3 +--
 net/sched/act_ipt.c        | 6 ++----
 net/sched/act_mirred.c     | 3 +--
 net/sched/act_nat.c        | 3 +--
 net/sched/act_pedit.c      | 3 +--
 net/sched/act_police.c     | 3 +--
 net/sched/act_sample.c     | 3 +--
 net/sched/act_simple.c     | 3 +--
 net/sched/act_skbedit.c    | 3 +--
 net/sched/act_skbmod.c     | 3 +--
 net/sched/act_tunnel_key.c | 3 +--
 net/sched/act_vlan.c       | 3 +--
 18 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 970303448c90..c6f195b3c706 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -85,8 +85,7 @@ struct tc_action_ops {
 		       struct tcf_result *); /* called under RCU BH lock*/
 	int     (*dump)(struct sk_buff *, struct tc_action *, int, int);
 	void	(*cleanup)(struct tc_action *);
-	int     (*lookup)(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack);
+	int     (*lookup)(struct net *net, struct tc_action **a, u32 index);
 	int     (*init)(struct net *net, struct nlattr *nla,
 			struct nlattr *est, struct tc_action **act, int ovr,
 			int bind, bool rtnl_held,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index db83dac1e7f4..398c752ff529 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1067,12 +1067,14 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
 	err = -EINVAL;
 	ops = tc_lookup_action(tb[TCA_ACT_KIND]);
 	if (!ops) { /* could happen in batch of actions */
-		NL_SET_ERR_MSG(extack, "Specified TC action not found");
+		NL_SET_ERR_MSG(extack, "Specified TC action kind not found");
 		goto err_out;
 	}
 	err = -ENOENT;
-	if (ops->lookup(net, &a, index, extack) == 0)
+	if (ops->lookup(net, &a, index) == 0) {
+		NL_SET_ERR_MSG(extack, "TC action with specified index not found");
 		goto err_mod;
+	}
 
 	module_put(ops->owner);
 	return a;
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 0c68bc9cf0b4..c7633843e223 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -387,8 +387,7 @@ static int tcf_bpf_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, bpf_net_id);
 
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 6f0f273f1139..e869c0ee63c8 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -190,8 +190,7 @@ static int tcf_connmark_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index,
-			       struct netlink_ext_ack *extack)
+static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, connmark_net_id);
 
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index b8a67ae3105a..3dc25b7806d7 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -646,8 +646,7 @@ static int tcf_csum_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, csum_net_id);
 
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index cd1d9bd32ef9..aa44d14b43c7 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -222,8 +222,7 @@ static int tcf_gact_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, gact_net_id);
 
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 196430aefe87..19454146f60d 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -841,8 +841,7 @@ static int tcf_ife_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, ife_net_id);
 
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 23273b5303fd..1efbfb10b1fc 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -329,8 +329,7 @@ static int tcf_ipt_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, ipt_net_id);
 
@@ -379,8 +378,7 @@ static int tcf_xt_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index,
-			 struct netlink_ext_ack *extack)
+static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, xt_net_id);
 
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 8bf66d0a6800..a9d64bfe5a2a 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -338,8 +338,7 @@ static int tcf_mirred_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, mirred_net_id);
 
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 4313aa102440..d98f33fdffe2 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -292,8 +292,7 @@ static int tcf_nat_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, nat_net_id);
 
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 107034070019..6d6a9450e8ad 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -452,8 +452,7 @@ static int tcf_pedit_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index,
-			    struct netlink_ext_ack *extack)
+static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, pedit_net_id);
 
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 5d8bfa878477..393c7a670300 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -312,8 +312,7 @@ static int tcf_police_dump(struct sk_buff *skb, struct tc_action *a,
 	return -1;
 }
 
-static int tcf_police_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_police_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, police_net_id);
 
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 44e9c00657bc..83a133375d6d 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -224,8 +224,7 @@ static int tcf_sample_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, sample_net_id);
 
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 52400d49f81f..902957beceb3 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -188,8 +188,7 @@ static int tcf_simp_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, simp_net_id);
 
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 73e44ce2a883..b6263704ea57 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -291,8 +291,7 @@ static int tcf_skbedit_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index,
-			      struct netlink_ext_ack *extack)
+static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, skbedit_net_id);
 
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 588077fafd6c..59710a183bd3 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -251,8 +251,7 @@ static int tcf_skbmod_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, skbmod_net_id);
 
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 420759153d5f..6d95b6919d9d 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -540,8 +540,7 @@ static int tunnel_key_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
 
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 033d273afe50..ba677d54a7af 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -288,8 +288,7 @@ static int tcf_vlan_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, vlan_net_id);
 
-- 
2.14.4

^ permalink raw reply related

* Re: [Bug #16626] Machine hangs with EIP at skb_copy_and_csum_dev
From: Mitchell Erblich @ 2018-08-29 21:39 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Plamen Petrov, Rafael J. Wysocki, Kernel Testers List,
	Maciej Rutecki, David S. Miller, Eric Dumazet,
	Linux Kernel Mailing List, netdev
In-Reply-To: <20100831192659.GA3093@del.dom.local>

						OLD REPLY … and Correcting . Suggesting of Corrections of LONG TERM embedded problems 

Summary:

		Due to watermark awareness differences that is problematic in embedded systems, the GFP_ATOMIC which is not memory watermark aware is used in interrupt / atomic context.

		To properly monitor WATERMARK levels at suggested kernel locations, the “PROPER” GFP_FLAG SHOULD be GFP_NOWAIT. This is atomic/interupt friendly and is aware of memory watermarks, thus if memory is below the specified watermark level, it will then return a ENOMEM from that location.

					GFP_ATOMIC will not return ENOMEM and by the time the watermarks drop, ALL GFP_KERNEL callers are now SLEEPING.

					Please be embedded friendly with code patches…

		FYI: Embedded system due to no 2ndary drive can not clean PTEs (page frames), thus callers need to be aware of low memory issues and be able to reduce their memory consumption based on receiving ENOMEMs. GFP_KERNEL callers will just sleep until memory is back above the watermarks.

Mitchell Erblich
erblichs@earthlink.net



> On Aug 31, 2010, at 12:26 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> 
> On Tue, Aug 31, 2010 at 08:58:10AM +0300, Plamen Petrov wrote:
>> Rafael J. Wysocki ????????????:
>> 
>>> This message has been generated automatically as a part of a summary report
>>> of recent regressions.
>>> 
>>> The following bug entry is on the current list of known regressions
>>> from 2.6.35.  Please verify if it still should be listed and let the tracking team
>>> know (either way).
>>> 
>>> 
>>> Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=16626
>>> Subject		: Machine hangs with EIP at skb_copy_and_csum_dev
>>> Submitter	: Plamen Petrov <pvp-lsts@fs.uni-ruse.bg>
>>> Date		: 2010-08-19 09:57 (11 days old)
>>> Handled-By	:  Eric Dumazet <eric.dumazet@gmail.com>
>>> 
>>> 
>> 
>> Should "generic receive offload" work on a forwarding setup?
>> If yes - then the bug should remain open.
>> If not - then it's my mistake.
> 
> If/since it's on by default it should work and it definitely can't be
> your mistake. (Unless we can't find the real reason... ;-)
> 
> Jarek P.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH bpf 0/3] Three fixes for bpf_msg_pull_data
From: Alexei Starovoitov @ 2018-08-29 17:52 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: john.fastabend, netdev
In-Reply-To: <20180829145036.5514-1-daniel@iogearbox.net>

On Wed, Aug 29, 2018 at 04:50:33PM +0200, Daniel Borkmann wrote:
> This set contains three more fixes for the bpf_msg_pull_data()
> mainly for correcting scatterlist ring wrap-arounds as well as
> fixing up data pointers. For details please see individual patches.
> Thanks!

Applied to bpf tree, Thanks

^ 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

* 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: [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: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: [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

* [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

* 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] 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

* 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

* 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


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