[PATCH v2 00/16] Protect code from pollers

[PATCH v2 00/16] Protect code from pollers

[Sascha Hauer]

Changes since v1:

- Do not recurse into dependencies during slice_acquire
- detect recursice dependencies
- Do not add the phy device as dependency to the ethernet device but
  instead lock them individually
- Add some more patches created during debugging this series

Sascha Hauer (16):
  net: fec_imx: Do not clear MII interrupt during receive
  miitool: Use mdiobus_read()
  net: phy: mdio-mux: Use mdiobus_read/write()
  net: Open ethernet devices explicitly
  Introduce slices
  net: Add a slice to struct eth_device
  net: mdiobus: Add slice
  usb: Add a slice to usb host controllers
  usbnet: Add slice
  net: Call net_poll() in a poller
  net: reply to ping requests
  usbnet: Be more friendly in the receive path
  net: phy: Also print link down messages
  net: ifup command: add ethernet device completion
  net: phy: Do not claim the link is up initially
  net: Add ifdown support and command

 commands/miitool.c         |   4 +-
 common/Makefile            |   1 +
 common/slice.c             | 323 +++++++++++++++++++++++++++++++++++++
 drivers/net/fec_imx.c      |   1 +
 drivers/net/phy/mdio-mux.c |   5 +-
 drivers/net/phy/mdio_bus.c |  40 +++++
 drivers/net/phy/phy.c      |   7 +-
 drivers/net/usb/usbnet.c   |  22 ++-
 drivers/usb/core/usb.c     |   6 +
 fs/nfs.c                   |   2 -
 fs/tftp.c                  |   2 -
 include/linux/phy.h        |  38 ++---
 include/net.h              |  18 ++-
 include/slice.h            |  31 ++++
 include/usb/usb.h          |   7 +
 include/usb/usbnet.h       |   3 +
 net/dhcp.c                 |   5 +-
 net/dns.c                  |   1 -
 net/eth.c                  |  82 ++++++----
 net/ifup.c                 |  75 +++++++++
 net/net.c                  |  60 ++++++-
 net/netconsole.c           |   4 +-
 net/nfs.c                  |   1 -
 net/ping.c                 |   2 -
 net/sntp.c                 |   2 -
 25 files changed, 657 insertions(+), 85 deletions(-)
 create mode 100644 common/slice.c
 create mode 100644 include/slice.h

-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 01/16] net: fec_imx: Do not clear MII interrupt during receive

[Sascha Hauer]

fec_recv() can be triggered during a MDIO access. so we may not clear
the FEC_IEVENT_MII interrupt the MDIO access is waiting for. Clearing
it in that moment causes the MDIO access to timeout.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/fec_imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
index 5ef1d4359e..f814b3b960 100644
--- a/drivers/net/fec_imx.c
+++ b/drivers/net/fec_imx.c
@@ -514,6 +514,7 @@ static int fec_recv(struct eth_device *dev)
 	 * Check if any critical events have happened
 	 */
 	ievent = readl(fec->regs + FEC_IEVENT);
+	ievent &= ~FEC_IEVENT_MII;
 	writel(ievent, fec->regs + FEC_IEVENT);
 
 	if (ievent & FEC_IEVENT_BABT) {
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 02/16] miitool: Use mdiobus_read()

[Sascha Hauer]

Use mdiobus_read() to read from the mii bus. Accessing the read hook
directly bypasses the acquiring the slice.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/miitool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commands/miitool.c b/commands/miitool.c
index a87d567ace..07437d28a8 100644
--- a/commands/miitool.c
+++ b/commands/miitool.c
@@ -110,10 +110,10 @@ static int show_basic_mii(struct mii_bus *mii, struct phy_device *phydev,
 
 	/* Some bits in the BMSR are latched, but we can't rely on being
 	   the only reader, so only the current values are meaningful */
-	mii->read(mii, phydev->addr, MII_BMSR);
+	mdiobus_read(mii, phydev->addr, MII_BMSR);
 
 	for (i = 0; i < 32; i++)
-		mii_val[i] = mii->read(mii, phydev->addr, i);
+		mii_val[i] = mdiobus_read(mii, phydev->addr, i);
 
 	printf((mii->parent->id) < 0 ? "%s: %s: " : "%s: %s%d: ",
 	       phydev->cdev.name, mii->parent->name, mii->parent->id);
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 03/16] net: phy: mdio-mux: Use mdiobus_read/write()

[Sascha Hauer]

Using the callbacks directly bypasses acquiring the slice.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/phy/mdio-mux.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index aa63cbde97..1e6278ef35 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -37,11 +37,10 @@ static int mdio_mux_read_or_write(struct mii_bus *bus, int phy_id,
 	if (!r) {
 		pb->current_child = cb->bus_number;
 		if (val)
-			r = pb->mii_bus->write(pb->mii_bus, phy_id,
+			r = mdiobus_write (pb->mii_bus, phy_id,
 					       regnum, *val);
 		else
-			r = pb->mii_bus->read(pb->mii_bus, phy_id,
-					      regnum);
+			r = mdiobus_read(pb->mii_bus, phy_id, regnum);
 	}
 	return r;
 }
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 04/16] net: Open ethernet devices explicitly

[Sascha Hauer]

Open ethernet devices explicitly rather than implicitly when sending
packets. This allows us to not only enable, but in the next step to
also disable ethernet devices.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  3 ++-
 net/dhcp.c    |  4 ++++
 net/eth.c     | 55 ++++++++++++++++++++++++++-------------------------
 net/ifup.c    |  4 ++++
 4 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/include/net.h b/include/net.h
index 6912a557b5..509d1b38a1 100644
--- a/include/net.h
+++ b/include/net.h
@@ -80,7 +80,8 @@ static inline const char *eth_name(struct eth_device *edev)
 int eth_register(struct eth_device* dev);    /* Register network device		*/
 void eth_unregister(struct eth_device* dev); /* Unregister network device	*/
 int eth_set_ethaddr(struct eth_device *edev, const char *ethaddr);
-
+int eth_open(struct eth_device *edev);
+void eth_close(struct eth_device *edev);
 int eth_send(struct eth_device *edev, void *packet, int length);	   /* Send a packet		*/
 int eth_rx(void);			/* Check for received packets	*/
 
diff --git a/net/dhcp.c b/net/dhcp.c
index 670a6a6422..a27fa89996 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -609,6 +609,10 @@ int dhcp(struct eth_device *edev, const struct dhcp_req_param *param)
 	struct dhcp_result *res;
 	int ret;
 
+	ret = eth_open(edev);
+	if (ret)
+		return ret;
+
 	ret = dhcp_request(edev, param, &res);
 	if (ret)
 		return ret;
diff --git a/net/eth.c b/net/eth.c
index 53d24baa16..4a8a7a8876 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -211,33 +211,12 @@ static int eth_carrier_check(struct eth_device *edev, int force)
 	return edev->phydev->link ? 0 : -ENETDOWN;
 }
 
-/*
- * Check if we have a current ethernet device and
- * eventually open it if we have to.
- */
-static int eth_check_open(struct eth_device *edev)
-{
-	int ret;
-
-	if (edev->active)
-		return 0;
-
-	ret = edev->open(edev);
-	if (ret)
-		return ret;
-
-	edev->active = 1;
-
-	return eth_carrier_check(edev, 1);
-}
-
 int eth_send(struct eth_device *edev, void *packet, int length)
 {
 	int ret;
 
-	ret = eth_check_open(edev);
-	if (ret)
-		return ret;
+	if (!edev->active)
+		return -ENETDOWN;
 
 	ret = eth_carrier_check(edev, 0);
 	if (ret)
@@ -252,10 +231,6 @@ static int __eth_rx(struct eth_device *edev)
 {
 	int ret;
 
-	ret = eth_check_open(edev);
-	if (ret)
-		return ret;
-
 	ret = eth_carrier_check(edev, 0);
 	if (ret)
 		return ret;
@@ -422,6 +397,32 @@ int eth_register(struct eth_device *edev)
 	return 0;
 }
 
+int eth_open(struct eth_device *edev)
+{
+	int ret;
+
+	if (edev->active)
+		return 0;
+
+	ret = edev->open(edev);
+	if (!ret)
+		edev->active = 1;
+
+	eth_carrier_check(edev, 1);
+
+	return ret;
+}
+
+void eth_close(struct eth_device *edev)
+{
+	if (!edev->active)
+		return;
+
+	edev->halt(edev);
+
+	edev->active = 0;
+}
+
 void eth_unregister(struct eth_device *edev)
 {
 	if (edev->active)
diff --git a/net/ifup.c b/net/ifup.c
index d550f82530..e5e8ef2346 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -214,6 +214,10 @@ int ifup_edev(struct eth_device *edev, unsigned flags)
 	if (ret)
 		return ret;
 
+	ret = eth_open(edev);
+	if (ret)
+		return ret;
+
 	if (edev->global_mode == ETH_MODE_DHCP) {
 		if (IS_ENABLED(CONFIG_NET_DHCP)) {
 			ret = dhcp(edev, NULL);
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 05/16] Introduce slices

[Sascha Hauer]

slices, the barebox idea of locking

barebox has pollers which execute code in the background whenever one of the
delay functions (udelay, mdelay, ...) or is_timeout() are called. This
introduces resource problems when some device triggers a poller by calling
a delay function and then the poller code calls into the same device again.

As an example consider a I2C GPIO expander which drives a LED which shall
be used as a heartbeat LED:

poller -> LED on/off -> GPIO high/low -> I2C transfer

The I2C controller has a timeout loop using is_timeout() and thus can trigger
a poller run. With this the following can happen during an unrelated I2C
transfer:

I2C transfer -> is_timeout() -> poller -> LED on/off -> GPIO high/low -> I2C transfer

We end up with issuing an I2C transfer during another I2C transfer and
things go downhill.

Due to the lack of interrupts we can't do real locking in barebox. We use
a mechanism called slices instead. A slice describes a resource to which
other slices can be attached. Whenever a slice is needed it must be acquired.
Acquiring a slice never fails, it just increases the acquired counter of
the slice and its dependent slices. when a slice shall be used inside a
poller it must first be tested if the slice is already in use. If it is,
we can't do the operation on the slice now and must return and hope that
we have more luck in the next poller call.

slices can be attached other slices as dependencies. In the example above
LED driver would add a dependency to the GPIO controller and the GPIO driver
would add a dependency to the I2C bus:

GPIO driver probe:

slice_add(&gpio->slice, i2c_device_slice(i2cdev));

LED driver probe:

slice_add(&led->slice, gpio_slice(gpio));

The GPIO code would call slice_acquire(&gpio->slice) before doing any
operation on the GPIO chip providing this GPIO, likewise the I2C core
would call slice_acquire(&i2cbus->slice) before doing an operation on
this I2C bus.

The heartbeat poller code would call slice_acquired(led_slice(led)) and
only continue when the slice is not acquired.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/Makefile |   1 +
 common/slice.c  | 323 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/slice.h |  31 +++++
 3 files changed, 355 insertions(+)
 create mode 100644 common/slice.c
 create mode 100644 include/slice.h

diff --git a/common/Makefile b/common/Makefile
index 84463b4d48..16f14db41c 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -11,6 +11,7 @@ obj-y				+= bootsource.o
 obj-$(CONFIG_ELF)		+= elf.o
 obj-y				+= restart.o
 obj-y				+= poweroff.o
+obj-y				+= slice.o
 obj-$(CONFIG_MACHINE_ID)	+= machine_id.o
 obj-$(CONFIG_AUTO_COMPLETE)	+= complete.o
 obj-y				+= version.o
diff --git a/common/slice.c b/common/slice.c
new file mode 100644
index 0000000000..5a37dc611d
--- /dev/null
+++ b/common/slice.c
@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt) "slice: " fmt
+
+#include <common.h>
+#include <slice.h>
+
+/*
+ * slices, the barebox idea of locking
+ *
+ * barebox has pollers which execute code in the background whenever one of the
+ * delay functions (udelay, mdelay, ...) or is_timeout() are called. This
+ * introduces resource problems when some device triggers a poller by calling
+ * a delay function and then the poller code calls into the same device again.
+ *
+ * As an example consider a I2C GPIO expander which drives a LED which shall
+ * be used as a heartbeat LED:
+ *
+ * poller -> LED on/off -> GPIO high/low -> I2C transfer
+ *
+ * The I2C controller has a timeout loop using is_timeout() and thus can trigger
+ * a poller run. With this the following can happen during an unrelated I2C
+ * transfer:
+ *
+ * I2C transfer -> is_timeout() -> poller -> LED on/off -> GPIO high/low -> I2C transfer
+ *
+ * We end up with issuing an I2C transfer during another I2C transfer and
+ * things go downhill.
+ *
+ * Due to the lack of interrupts we can't do real locking in barebox. We use
+ * a mechanism called slices instead. A slice describes a resource to which
+ * other slices can be attached. Whenever a slice is needed it must be acquired.
+ * Acquiring a slice never fails, it just increases the acquired counter of
+ * the slice and its dependent slices. when a slice shall be used inside a
+ * poller it must first be tested if the slice is already in use. If it is,
+ * we can't do the operation on the slice now and must return and hope that
+ * we have more luck in the next poller call.
+ *
+ * slices can be attached other slices as dependencies. In the example above
+ * LED driver would add a dependency to the GPIO controller and the GPIO driver
+ * would add a dependency to the I2C bus:
+ *
+ * GPIO driver probe:
+ *
+ * slice_add(&gpio->slice, i2c_device_slice(i2cdev));
+ *
+ * LED driver probe:
+ *
+ * slice_add(&led->slice, gpio_slice(gpio));
+ *
+ * The GPIO code would call slice_acquire(&gpio->slice) before doing any
+ * operation on the GPIO chip providing this GPIO, likewise the I2C core
+ * would call slice_acquire(&i2cbus->slice) before doing an operation on
+ * this I2C bus.
+ *
+ * The heartbeat poller code would call slice_acquired(led_slice(led)) and
+ * only continue when the slice is not acquired.
+ */
+
+/*
+ * slices are not required to have a device and thus a name, but it really
+ * helps debugging when it has one.
+ */
+static const char *slice_name(struct slice *slice)
+{
+	return slice->dev ? dev_name(slice->dev) : "<unknown>";
+}
+
+static void __slice_acquire(struct slice *slice, int add)
+{
+	slice->acquired += add;
+
+	if (slice->acquired < 0) {
+		pr_err("Slice %s acquired count drops below zero\n",
+			slice_name(slice));
+		dump_stack();
+		slice->acquired = 0;
+		return;
+	}
+}
+
+/**
+ * slice_acquire: acquire a slice
+ * @slice: The slice to acquire
+ *
+ * This acquires a slice and its dependencies
+ */
+void slice_acquire(struct slice *slice)
+{
+	__slice_acquire(slice, 1);
+}
+
+/**
+ * slice_release: release a slice
+ * @slice: The slice to release
+ *
+ * This releases a slice and its dependencies
+ */
+void slice_release(struct slice *slice)
+{
+	__slice_acquire(slice, -1);
+}
+
+/**
+ * slice_acquired: test if a slice is acquired
+ * @slice: The slice to test
+ *
+ * This tests if a slice is acquired. Returns true if it is, false otherwise
+ */
+bool slice_acquired(struct slice *slice)
+{
+	struct slice_entry *se;
+	int acquired = slice->acquired;
+	bool ret = false;
+
+	if (acquired > 0)
+		return true;
+
+	if (acquired < 0) {
+		pr_err("Recursive dependency detected in slice %s\n",
+		       slice_name(slice));
+		panic("Cannot continue");
+	}
+
+	slice->acquired = -1;
+
+	list_for_each_entry(se, &slice->deps, list)
+		if (slice_acquired(se->slice)) {
+			ret = true;
+			break;
+		}
+
+	slice->acquired = acquired;
+
+	return ret;
+}
+
+static void __slice_debug_acquired(struct slice *slice, int level)
+{
+	struct slice_entry *se;
+
+	pr_debug("%*s%s: %d\n", level * 4, "",
+		 slice_name(slice),
+		 slice->acquired);
+	
+	list_for_each_entry(se, &slice->deps, list)
+		__slice_debug_acquired(se->slice, level + 1);
+}
+
+/**
+ * slice_debug_acquired: print the acquired state of a slice
+ *
+ * @slice: The slice to print
+ *
+ * This prints the acquired state of a slice and its dependencies.
+ */
+void slice_debug_acquired(struct slice *slice)
+{
+	if (!slice_acquired(slice))
+		return;
+
+	__slice_debug_acquired(slice, 0);
+}
+
+/**
+ * slice_add: Add a dependency to a slice
+ *
+ * @slice: The slice to add the dependency to
+ * @dep:   The slice @slice depends on
+ *
+ * This makes @slice dependent on @dep. In other words, acquiring @slice
+ * will lead to also acquiring @dep.
+ */
+void slice_add(struct slice *slice, struct slice *dep)
+{
+	struct slice_entry *se = xzalloc(sizeof(*se));
+
+	pr_debug("Adding dependency %s (%d) to slice %s (%d)\n",
+		 slice_name(dep), dep->acquired,
+		 slice_name(slice), slice->acquired);
+
+	se->slice = dep;
+
+	list_add_tail(&se->list, &slice->deps);
+}
+
+static LIST_HEAD(slices);
+
+/**
+ * slice_del: Remove a dependency previously added with slice_add
+ * @slice: The slice to remove the dependency from
+ * @dep:   The slice @slice depended on
+ */
+void slice_del(struct slice *slice, struct slice *dep)
+{
+	struct slice_entry *se;
+
+	list_del(&slice->list);
+
+	list_for_each_entry(se, &slice->deps, list) {
+		if (se->slice == dep) {
+			list_del(&se->list);
+			free(se);
+			return;
+		}
+	}
+}
+
+/**
+ * slice_init - initialize a slice before usage
+ * @slice: The slice to initialize
+ * @dev:   a device as context pointer
+ *
+ * This initializes a slice before usage. Passing a nonzero @dev pointer
+ * is optional but strongly recommended to allow printing some context
+ * when dumping the dependencies.
+ */
+void slice_init(struct slice *slice, struct device_d *dev)
+{
+	INIT_LIST_HEAD(&slice->deps);
+	slice->dev = dev;
+	list_add_tail(&slice->list, &slices);
+}
+
+#ifdef DEBUG
+#include <command.h>
+#include <getopt.h>
+
+static void slice_info(void)
+{
+	struct slice *slice;
+	struct slice_entry *se;
+
+	list_for_each_entry(slice, &slices, list) {
+		printf("slice %s: acquired=%d\n",
+		       slice_name(slice), slice->acquired);
+		list_for_each_entry(se, &slice->deps, list)
+			printf("    dep: %s\n", slice_name(se->slice));
+	}
+}
+
+static int __dev_acquire(const char *devname, int add)
+{
+	struct device_d *dev;
+	struct slice *slice;
+
+	dev = get_device_by_name(optarg);
+	if (!dev) {
+		pr_err("No such device: %s\n", optarg);
+		return 1;
+	}
+
+	list_for_each_entry(slice, &slices, list) {
+		if (slice->dev == dev) {
+			__slice_acquire(slice, add);
+			return 0;
+		}
+	}
+
+	printf("No slice for %s\n", optarg);
+
+	return 1;
+}
+
+static void slice_time(void)
+{
+	uint64_t start = get_time_ns();
+	int i = 0;
+
+	/*
+	 * Not really slice related, but useful to know in this context:
+	 * How many times we can run the registered pollers in one second?
+	 *
+	 * A low number here may point to problems with pollers taking too
+	 * much time.
+	 */
+	while (!is_timeout(start, SECOND))
+		i++;
+
+	printf("%d poller calls in 1s\n", i);
+}
+
+BAREBOX_CMD_HELP_START(slice)
+BAREBOX_CMD_HELP_TEXT("slice debugging command")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-i", "Print information about slices")
+BAREBOX_CMD_HELP_OPT ("-a <devname>", "acquire a slice")
+BAREBOX_CMD_HELP_OPT ("-r <devname>", "release a slice")
+BAREBOX_CMD_HELP_OPT ("-t", "measure how many pollers we run in 1s")
+BAREBOX_CMD_HELP_END
+
+static int do_slice(int argc, char *argv[])
+{
+	int opt;
+
+	while ((opt = getopt(argc, argv, "a:r:it")) > 0) {
+		switch (opt) {
+		case 'a':
+			return __dev_acquire(optarg, 1);
+		case 'r':
+			return __dev_acquire(optarg, -1);
+		case 'i':
+			slice_info();
+			return 0;
+		case 't':
+			slice_time();
+			return 0;
+		}
+	}
+
+	return COMMAND_ERROR_USAGE;
+}
+
+BAREBOX_CMD_START(slice)
+	.cmd = do_slice,
+	BAREBOX_CMD_DESC("debug slices")
+        BAREBOX_CMD_OPTS("[-ari]")
+        BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+        BAREBOX_CMD_HELP(cmd_slice_help)
+
+BAREBOX_CMD_END
+#endif
diff --git a/include/slice.h b/include/slice.h
new file mode 100644
index 0000000000..e8fdde17c4
--- /dev/null
+++ b/include/slice.h
@@ -0,0 +1,31 @@
+#ifndef __SLICE_H
+#define __SLICE_H
+
+enum slice_action {
+	SLICE_ACQUIRE = 1,
+	SLICE_RELEASE = -1,
+	SLICE_TEST = 0,
+};
+
+struct slice {
+	int acquired;
+	struct list_head deps;
+	struct device_d *dev;
+	struct list_head list;
+};
+
+struct slice_entry {
+	struct slice *slice;
+	struct list_head list;
+};
+
+void slice_acquire(struct slice *slice);
+void slice_release(struct slice *slice);
+bool slice_acquired(struct slice *slice);
+void slice_add(struct slice *slice, struct slice *dep);
+void slice_del(struct slice *slice, struct slice *dep);
+void slice_init(struct slice *slice, struct device_d *dev);
+
+void slice_debug_acquired(struct slice *slice);
+
+#endif /* __SLICE_H */
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 05/16] Introduce slices

[Daniel Glöckner]

Hello Sascha,

On Wed, Mar 11, 2020 at 03:27:46PM +0100, Sascha Hauer wrote:
> +/**
> + * slice_acquired: test if a slice is acquired
> + * @slice: The slice to test
> + *
> + * This tests if a slice is acquired. Returns true if it is, false otherwise
> + */
> +bool slice_acquired(struct slice *slice)
> +{
> +	struct slice_entry *se;
> +	int acquired = slice->acquired;
> +	bool ret = false;
> +
> +	if (acquired > 0)
> +		return true;
> +
> +	if (acquired < 0) {
> +		pr_err("Recursive dependency detected in slice %s\n",
> +		       slice_name(slice));
> +		panic("Cannot continue");
> +	}
> +
> +	slice->acquired = -1;
> +
> +	list_for_each_entry(se, &slice->deps, list)
> +		if (slice_acquired(se->slice)) {
> +			ret = true;
> +			break;
> +		}
> +
> +	slice->acquired = acquired;

no need to restore slice->acquire from acquired since
acquired must have been zero if we reach this line.

Best regards,

  Daniel

-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 05/16] Introduce slices

[Sascha Hauer]

On Thu, Mar 12, 2020 at 12:51:54AM +0100, Daniel Glöckner wrote:
> Hello Sascha,
> 
> On Wed, Mar 11, 2020 at 03:27:46PM +0100, Sascha Hauer wrote:
> > +/**
> > + * slice_acquired: test if a slice is acquired
> > + * @slice: The slice to test
> > + *
> > + * This tests if a slice is acquired. Returns true if it is, false otherwise
> > + */
> > +bool slice_acquired(struct slice *slice)
> > +{
> > +	struct slice_entry *se;
> > +	int acquired = slice->acquired;
> > +	bool ret = false;
> > +
> > +	if (acquired > 0)
> > +		return true;
> > +
> > +	if (acquired < 0) {
> > +		pr_err("Recursive dependency detected in slice %s\n",
> > +		       slice_name(slice));
> > +		panic("Cannot continue");
> > +	}
> > +
> > +	slice->acquired = -1;
> > +
> > +	list_for_each_entry(se, &slice->deps, list)
> > +		if (slice_acquired(se->slice)) {
> > +			ret = true;
> > +			break;
> > +		}
> > +
> > +	slice->acquired = acquired;
> 
> no need to restore slice->acquire from acquired since
> acquired must have been zero if we reach this line.

Hm, no. Nothing changes slice->acquired in this function besides
ourselves. It is still -1 how we have set it previously.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 05/16] Introduce slices

[Daniel Glöckner]

On Thu, Mar 12, 2020 at 09:24:59AM +0100, Sascha Hauer wrote:
> On Thu, Mar 12, 2020 at 12:51:54AM +0100, Daniel Glöckner wrote:
> > Hello Sascha,
> > 
> > On Wed, Mar 11, 2020 at 03:27:46PM +0100, Sascha Hauer wrote:
> > > +/**
> > > + * slice_acquired: test if a slice is acquired
> > > + * @slice: The slice to test
> > > + *
> > > + * This tests if a slice is acquired. Returns true if it is, false otherwise
> > > + */
> > > +bool slice_acquired(struct slice *slice)
> > > +{
> > > +	struct slice_entry *se;
> > > +	int acquired = slice->acquired;
> > > +	bool ret = false;
> > > +
> > > +	if (acquired > 0)
> > > +		return true;
> > > +
> > > +	if (acquired < 0) {
> > > +		pr_err("Recursive dependency detected in slice %s\n",
> > > +		       slice_name(slice));
> > > +		panic("Cannot continue");
> > > +	}
> > > +
> > > +	slice->acquired = -1;
> > > +
> > > +	list_for_each_entry(se, &slice->deps, list)
> > > +		if (slice_acquired(se->slice)) {
> > > +			ret = true;
> > > +			break;
> > > +		}
> > > +
> > > +	slice->acquired = acquired;
> > 
> > no need to restore slice->acquire from acquired since
> > acquired must have been zero if we reach this line.
> 
> Hm, no. Nothing changes slice->acquired in this function besides
> ourselves. It is still -1 how we have set it previously.

What I wanted to say is that you can simply use
	slice->acquired = 0;
at the end.

Best regards,

  Daniel


-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 05/16] Introduce slices

[Sascha Hauer]

On Thu, Mar 12, 2020 at 10:18:10AM +0100, Daniel Glöckner wrote:
> On Thu, Mar 12, 2020 at 09:24:59AM +0100, Sascha Hauer wrote:
> > On Thu, Mar 12, 2020 at 12:51:54AM +0100, Daniel Glöckner wrote:
> > > Hello Sascha,
> > > 
> > > On Wed, Mar 11, 2020 at 03:27:46PM +0100, Sascha Hauer wrote:
> > > > +/**
> > > > + * slice_acquired: test if a slice is acquired
> > > > + * @slice: The slice to test
> > > > + *
> > > > + * This tests if a slice is acquired. Returns true if it is, false otherwise
> > > > + */
> > > > +bool slice_acquired(struct slice *slice)
> > > > +{
> > > > +	struct slice_entry *se;
> > > > +	int acquired = slice->acquired;
> > > > +	bool ret = false;
> > > > +
> > > > +	if (acquired > 0)
> > > > +		return true;
> > > > +
> > > > +	if (acquired < 0) {
> > > > +		pr_err("Recursive dependency detected in slice %s\n",
> > > > +		       slice_name(slice));
> > > > +		panic("Cannot continue");
> > > > +	}
> > > > +
> > > > +	slice->acquired = -1;
> > > > +
> > > > +	list_for_each_entry(se, &slice->deps, list)
> > > > +		if (slice_acquired(se->slice)) {
> > > > +			ret = true;
> > > > +			break;
> > > > +		}
> > > > +
> > > > +	slice->acquired = acquired;
> > > 
> > > no need to restore slice->acquire from acquired since
> > > acquired must have been zero if we reach this line.
> > 
> > Hm, no. Nothing changes slice->acquired in this function besides
> > ourselves. It is still -1 how we have set it previously.
> 
> What I wanted to say is that you can simply use
> 	slice->acquired = 0;
> at the end.

Yes, in the meantime a collegue explained that to me as well ;)
Changed accordingly.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 06/16] net: Add a slice to struct eth_device

[Sascha Hauer]

Add ethernet code safe for being called from a poller.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  8 ++++++++
 net/eth.c     | 20 +++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/include/net.h b/include/net.h
index 509d1b38a1..70d3ce2849 100644
--- a/include/net.h
+++ b/include/net.h
@@ -19,6 +19,7 @@
 #include <stdlib.h>
 #include <clock.h>
 #include <led.h>
+#include <slice.h>
 #include <xfuncs.h>
 #include <linux/phy.h>
 #include <linux/string.h>	/* memcpy */
@@ -63,6 +64,8 @@ struct eth_device {
 	char *bootarg;
 	char *linuxdevname;
 
+	struct slice slice;
+
 	bool ifup;
 #define ETH_MODE_DHCP 0
 #define ETH_MODE_STATIC 1
@@ -72,6 +75,11 @@ struct eth_device {
 
 #define dev_to_edev(d) container_of(d, struct eth_device, dev)
 
+static inline struct slice *eth_device_slice(struct eth_device *edev)
+{
+	return &edev->slice;
+}
+
 static inline const char *eth_name(struct eth_device *edev)
 {
 	return edev->devname;
diff --git a/net/eth.c b/net/eth.c
index 4a8a7a8876..59972b8949 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -222,20 +222,32 @@ int eth_send(struct eth_device *edev, void *packet, int length)
 	if (ret)
 		return ret;
 
+	slice_acquire(eth_device_slice(edev));
+
 	led_trigger_network(LED_TRIGGER_NET_TX);
 
-	return edev->send(edev, packet, length);
+	ret = edev->send(edev, packet, length);
+
+	slice_release(eth_device_slice(edev));
+
+	return ret;
 }
 
 static int __eth_rx(struct eth_device *edev)
 {
 	int ret;
 
+	slice_acquire(eth_device_slice(edev));
+
 	ret = eth_carrier_check(edev, 0);
 	if (ret)
-		return ret;
+		goto out;
+
+	ret = edev->recv(edev);
+out:
+	slice_release(eth_device_slice(edev));
 
-	return edev->recv(edev);
+	return ret;
 }
 
 int eth_rx(void)
@@ -352,6 +364,8 @@ int eth_register(struct eth_device *edev)
 		edev->dev.id = DEVICE_ID_DYNAMIC;
 	}
 
+	slice_init(&edev->slice, dev);
+
 	ret = register_device(&edev->dev);
 	if (ret)
 		return ret;
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 07/16] net: mdiobus: Add slice

[Sascha Hauer]

By adding a slice to the mdio bus we make the mdio code safe for being
called in a poller.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/phy/mdio_bus.c | 40 ++++++++++++++++++++++++++++++++++++++
 include/linux/phy.h        | 38 ++++++++++++++----------------------
 2 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 3480e2ffb4..30c2e1f409 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -232,6 +232,7 @@ int mdiobus_register(struct mii_bus *bus)
 	dev_set_name(&bus->dev, "miibus");
 	bus->dev.parent = bus->parent;
 	bus->dev.detect = mdiobus_detect;
+	slice_init(&bus->slice, &bus->dev);
 
 	err = register_device(&bus->dev);
 	if (err) {
@@ -357,6 +358,45 @@ static int mdio_bus_match(struct device_d *dev, struct driver_d *drv)
 	return 1;
 }
 
+/**
+ * mdiobus_read - Convenience function for reading a given MII mgmt register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to read
+ */
+int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
+{
+	int ret;
+
+	slice_acquire(&bus->slice);
+
+	ret = bus->read(bus, addr, regnum);
+
+	slice_release(&bus->slice);
+
+	return ret;
+}
+
+/**
+ * mdiobus_write - Convenience function for writing a given MII mgmt register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ */
+int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
+{
+	int ret;
+
+	slice_acquire(&bus->slice);
+
+	ret = bus->write(bus, addr, regnum, val);
+
+	slice_release(&bus->slice);
+
+	return ret;
+}
+
 static ssize_t phydev_read(struct cdev *cdev, void *_buf, size_t count, loff_t offset, ulong flags)
 {
 	int i = count;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index a9fdf44f1a..e93f6a01ff 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -16,6 +16,7 @@
 #define __PHY_H
 
 #include <driver.h>
+#include <slice.h>
 #include <linux/list.h>
 #include <linux/ethtool.h>
 #include <linux/mii.h>
@@ -116,9 +117,16 @@ struct mii_bus {
 	struct list_head list;
 
 	bool is_multiplexed;
+
+	struct slice slice;
 };
 #define to_mii_bus(d) container_of(d, struct mii_bus, dev)
 
+static inline struct slice *mdiobus_slice(struct mii_bus *bus)
+{
+	return &bus->slice;
+}
+
 int mdiobus_register(struct mii_bus *bus);
 void mdiobus_unregister(struct mii_bus *bus);
 struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
@@ -134,28 +142,8 @@ struct mii_bus *mdiobus_get_bus(int busnum);
 
 struct mii_bus *of_mdio_find_bus(struct device_node *mdio_bus_np);
 
-/**
- * mdiobus_read - Convenience function for reading a given MII mgmt register
- * @bus: the mii_bus struct
- * @addr: the phy address
- * @regnum: register number to read
- */
-static inline int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
-{
-	return bus->read(bus, addr, regnum);
-}
-
-/**
- * mdiobus_write - Convenience function for writing a given MII mgmt register
- * @bus: the mii_bus struct
- * @addr: the phy address
- * @regnum: register number to write
- * @val: value to write to @regnum
- */
-static inline int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
-{
-	return bus->write(bus, addr, regnum, val);
-}
+int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
+int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
 
 /* phy_device: An instance of a PHY
  *
@@ -376,11 +364,15 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
 int phy_register_fixup_for_id(const char *bus_id,
 		int (*run)(struct phy_device *));
 int phy_scan_fixups(struct phy_device *phydev);
-
 int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
 void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
 				   u16 data);
 
+static inline bool phy_acquired(struct phy_device *phydev)
+{
+	return phydev && slice_acquired(&phydev->bus->slice);
+}
+
 #ifdef CONFIG_PHYLIB
 int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
 		int (*run)(struct phy_device *));
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 08/16] usb: Add a slice to usb host controllers

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/core/usb.c | 6 ++++++
 include/usb/usb.h      | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 1c3dcb79a8..da7afb5a32 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -83,11 +83,16 @@ static inline int usb_host_acquire(struct usb_host *host)
 	if (host->sem)
 		return -EAGAIN;
 	host->sem++;
+
+	slice_acquire(&host->slice);
+
 	return 0;
 }
 
 static inline void usb_host_release(struct usb_host *host)
 {
+	slice_release(&host->slice);
+
 	if (host->sem > 0)
 		host->sem--;
 }
@@ -97,6 +102,7 @@ int usb_register_host(struct usb_host *host)
 	list_add_tail(&host->list, &host_list);
 	host->busnum = host_busnum++;
 	host->sem = 0;
+	slice_init(&host->slice, host->hw_dev);
 	return 0;
 }
 
diff --git a/include/usb/usb.h b/include/usb/usb.h
index 95dedfd5b7..d730578fd7 100644
--- a/include/usb/usb.h
+++ b/include/usb/usb.h
@@ -23,6 +23,7 @@
 #define _USB_H_
 
 #include <driver.h>
+#include <slice.h>
 #include <usb/ch9.h>
 #include <usb/ch11.h>
 #include <usb/usb_defs.h>
@@ -154,11 +155,17 @@ struct usb_host {
 	struct usb_device *root_dev;
 	int sem;
 	struct usb_phy *usbphy;
+	struct slice slice;
 };
 
 int usb_register_host(struct usb_host *);
 void usb_unregister_host(struct usb_host *host);
 
+static inline struct slice *usb_device_slice(struct usb_device *udev)
+{
+	return &udev->host->slice;
+}
+
 int usb_host_detect(struct usb_host *host);
 
 int usb_set_protocol(struct usb_device *dev, int ifnum, int protocol);
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 09/16] usbnet: Add slice

[Sascha Hauer]

Both the ethernet device and the mdio bus of a USB network controller
need the USB bus. Add dependencies to it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/usb/usbnet.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 60e67ff1a2..f1a49a142c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -213,6 +213,9 @@ int usbnet_probe(struct usb_device *usbdev, const struct usb_device_id *prod)
 
 	eth_register(edev);
 
+	slice_add(eth_device_slice(edev), usb_device_slice(usbdev));
+	slice_add(mdiobus_slice(&undev->miibus), usb_device_slice(usbdev));
+
 	return 0;
 out1:
 	dev_dbg(&edev->dev, "err: %d\n", status);
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/16] net: Call net_poll() in a poller

[Sascha Hauer]

This patch moves the ethernet receive loop into a poller. With this
network protocols no longer have to call net_loop() explicitly but
can do it implicitly by calling is_timeout() when waiting for packets.

Having the network receive loop running in a poller has the advantage
that we have it running continuously and not only when we explicitly
expect packets to come in. With this we can reasonably well answer to
ping requests which is implemented in the next patch. This also helps
protocols that run in the background like fastboot over UDP.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/nfs.c         |  2 --
 fs/tftp.c        |  2 --
 include/net.h    |  3 ---
 net/dhcp.c       |  1 -
 net/dns.c        |  1 -
 net/eth.c        | 15 ++++++++++-----
 net/net.c        | 14 +++++++++++---
 net/netconsole.c |  4 +---
 net/nfs.c        |  1 -
 net/ping.c       |  2 --
 net/sntp.c       |  2 --
 11 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/fs/nfs.c b/fs/nfs.c
index 0ad07aa3f2..355202df45 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -459,8 +459,6 @@ again:
 		if (ctrlc())
 			return ERR_PTR(-EINTR);
 
-		net_poll();
-
 		if (is_timeout(nfs_timer_start, NFS_TIMEOUT)) {
 			tries++;
 			if (tries == NFS_MAX_RESEND)
diff --git a/fs/tftp.c b/fs/tftp.c
index 1f36c56511..11f289584d 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -210,8 +210,6 @@ static int tftp_poll(struct file_priv *priv)
 		return -ETIMEDOUT;
 	}
 
-	net_poll();
-
 	return 0;
 }
 
diff --git a/include/net.h b/include/net.h
index 70d3ce2849..23e3759905 100644
--- a/include/net.h
+++ b/include/net.h
@@ -244,9 +244,6 @@ IPaddr_t net_get_nameserver(void);
 const char *net_get_domainname(void);
 struct eth_device *net_route(IPaddr_t ip);
 
-/* Do the work */
-void net_poll(void);
-
 static inline struct iphdr *net_eth_to_iphdr(char *pkt)
 {
 	return (struct iphdr *)(pkt + ETHER_HDR_SIZE);
diff --git a/net/dhcp.c b/net/dhcp.c
index a27fa89996..ef22d2cef0 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -513,7 +513,6 @@ int dhcp_request(struct eth_device *edev, const struct dhcp_req_param *param,
 			ret = -ETIMEDOUT;
 			goto out1;
 		}
-		net_poll();
 		if (is_timeout(dhcp_start, 3 * SECOND)) {
 			dhcp_start = get_time_ns();
 			printf("T ");
diff --git a/net/dns.c b/net/dns.c
index ffe98ef9e3..851bf3722e 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -231,7 +231,6 @@ int resolv(const char *host, IPaddr_t *ip)
 		if (ctrlc()) {
 			break;
 		}
-		net_poll();
 		if (is_timeout(dns_timer_start, SECOND)) {
 			dns_timer_start = get_time_ns();
 			printf("T ");
diff --git a/net/eth.c b/net/eth.c
index 59972b8949..4be205a096 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -237,14 +237,19 @@ static int __eth_rx(struct eth_device *edev)
 {
 	int ret;
 
-	slice_acquire(eth_device_slice(edev));
+	if (!phy_acquired(edev->phydev)) {
+		ret = eth_carrier_check(edev, 0);
+		if (ret)
+			return ret;
+	}
 
-	ret = eth_carrier_check(edev, 0);
-	if (ret)
-		goto out;
+	if (slice_acquired(eth_device_slice(edev)))
+		return 0;
+
+	slice_acquire(eth_device_slice(edev));
 
 	ret = edev->recv(edev);
-out:
+
 	slice_release(eth_device_slice(edev));
 
 	return ret;
diff --git a/net/net.c b/net/net.c
index 0d889ddb52..a5c7fb5211 100644
--- a/net/net.c
+++ b/net/net.c
@@ -242,8 +242,6 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 
 		if (retries > PKT_NUM_RETRIES)
 			return -ETIMEDOUT;
-
-		net_poll();
 	}
 
 	pr_debug("Got ARP REPLY for %pI4: %02x:%02x:%02x:%02x:%02x:%02x\n",
@@ -252,11 +250,21 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 	return 0;
 }
 
-void net_poll(void)
+static void net_poll(struct poller_struct *poller)
 {
 	eth_rx();
 }
 
+static struct poller_struct net_poller = {
+	.func = net_poll,
+};
+
+static int init_net_poll(void)
+{
+	return poller_register(&net_poller);
+}
+device_initcall(init_net_poll);
+
 static uint16_t net_udp_new_localport(void)
 {
 	static uint16_t localport;
diff --git a/net/netconsole.c b/net/netconsole.c
index ef8b1856b6..457a3c2971 100644
--- a/net/netconsole.c
+++ b/net/netconsole.c
@@ -62,7 +62,7 @@ static int nc_getc(struct console_device *cdev)
 		return 0;
 
 	while (!kfifo_len(priv->fifo))
-		net_poll();
+		udelay(1);
 
 	kfifo_getc(priv->fifo, &c);
 
@@ -80,8 +80,6 @@ static int nc_tstc(struct console_device *cdev)
 	if (priv->busy)
 		return kfifo_len(priv->fifo) ? 1 : 0;
 
-	net_poll();
-
 	return kfifo_len(priv->fifo) ? 1 : 0;
 }
 
diff --git a/net/nfs.c b/net/nfs.c
index 63573098d7..e0479ef692 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -708,7 +708,6 @@ static int do_nfs(int argc, char *argv[])
 			nfs_err = -EINTR;
 			break;
 		}
-		net_poll();
 		if (is_timeout(nfs_timer_start, NFS_TIMEOUT * SECOND)) {
 			show_progress(-1);
 			nfs_send();
diff --git a/net/ping.c b/net/ping.c
index a71f59a805..f3de0c27a4 100644
--- a/net/ping.c
+++ b/net/ping.c
@@ -89,8 +89,6 @@ static int do_ping(int argc, char *argv[])
 			break;
 		}
 
-		net_poll();
-
 		if (is_timeout(ping_start, SECOND)) {
 			/* No answer, send another packet */
 			ping_start = get_time_ns();
diff --git a/net/sntp.c b/net/sntp.c
index b4e6d6439c..f693a2e8af 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -145,8 +145,6 @@ s64 sntp(const char *server)
 			break;
 		}
 
-		net_poll();
-
 		if (is_timeout(sntp_start, 1 * SECOND)) {
 			sntp_start = get_time_ns();
 			ret = sntp_send();
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 11/16] net: reply to ping requests

[Sascha Hauer]

Now that we have the network receive function running in a poller we
can reasonably well answer to ping requests. Implement this feature.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/net.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/net/net.c b/net/net.c
index a5c7fb5211..abb0a1e5e7 100644
--- a/net/net.c
+++ b/net/net.c
@@ -573,12 +573,54 @@ static int net_handle_udp(unsigned char *pkt, int len)
 	return -EINVAL;
 }
 
-static int net_handle_icmp(unsigned char *pkt, int len)
+static int ping_reply(struct eth_device *edev, unsigned char *pkt, int len)
+{
+	struct ethernet *et = (struct ethernet *)pkt;
+	struct icmphdr *icmp;
+	struct iphdr *ip = (struct iphdr *)(pkt + ETHER_HDR_SIZE);
+	unsigned char *packet;
+	int ret;
+
+	memcpy(et->et_dest, et->et_src, 6);
+	memcpy(et->et_src, edev->ethaddr, 6);
+	et->et_protlen = htons(PROT_IP);
+
+	icmp = net_eth_to_icmphdr(pkt);
+
+	icmp->type = ICMP_ECHO_REPLY;
+	icmp->checksum = 0;
+	icmp->checksum = ~net_checksum((unsigned char *)icmp,
+				       len - sizeof(struct iphdr) - ETHER_HDR_SIZE);
+	ip->check = 0;
+	ip->frag_off = 0;
+	net_copy_ip((void *)&ip->daddr, &ip->saddr);
+	net_copy_ip((void *)&ip->saddr, &edev->ipaddr);
+	ip->check = ~net_checksum((unsigned char *)ip, sizeof(struct iphdr));
+
+	packet = net_alloc_packet();
+	if (!packet)
+		return 0;
+
+	memcpy(packet, pkt, ETHER_HDR_SIZE + len);
+
+	ret = eth_send(edev, packet, ETHER_HDR_SIZE + len);
+
+	free(packet);
+
+	return 0;
+}
+
+static int net_handle_icmp(struct eth_device *edev, unsigned char *pkt, int len)
 {
 	struct net_connection *con;
+	struct icmphdr *icmp;
 
 	pr_debug("%s\n", __func__);
 
+	icmp = net_eth_to_icmphdr(pkt);
+	if (icmp->type == ICMP_ECHO_REQUEST)
+		ping_reply(edev, pkt, len);
+
 	list_for_each_entry(con, &connection_list, list) {
 		if (con->proto == IPPROTO_ICMP) {
 			con->handler(con->priv, pkt, len);
@@ -615,7 +657,7 @@ static int net_handle_ip(struct eth_device *edev, unsigned char *pkt, int len)
 
 	switch (ip->protocol) {
 	case IPPROTO_ICMP:
-		return net_handle_icmp(pkt, len);
+		return net_handle_icmp(edev, pkt, len);
 	case IPPROTO_UDP:
 		return net_handle_udp(pkt, len);
 	}
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 12/16] usbnet: Be more friendly in the receive path

[Sascha Hauer]

To recognize if we have a receive packet pending we must set up a USB
bulk transfer. When there's no incoming packet we must wait until the
transfer times out. We do this with every poller call which can
considerably slow down the system. With this patch we do two things
against this:

- lower the timeout for the bulk transfer
- When we haven't received a packet for longer then lower the frequency
  of calling into the USB stack to once every 100ms

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/usb/usbnet.c | 19 ++++++++++++++++++-
 include/usb/usbnet.h     |  3 +++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index f1a49a142c..38aa73047e 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -123,12 +123,29 @@ static int usbnet_recv(struct eth_device *edev)
 
 	dev_dbg(&edev->dev, "%s\n",__func__);
 
+	/*
+	 * we must let the usb_bulk_msg below timeout before we realize
+	 * that we have no packet received. Since this function runs
+	 * inside a poller we considerably slow down barebox when we
+	 * wait for the timeout too often. To improve this we only poll
+	 * with full speed when we actually have received a packet in the
+	 * last 100ms.
+	 */
+	if (is_timeout(dev->last_pkt_received, 100 * MSECOND) &&
+	    !is_timeout(dev->last_recv_call, 100 * MSECOND)) {
+		return 0;
+	}
+
+	dev->last_recv_call = get_time_ns();
+
 	len = dev->rx_urb_size;
 
-	ret = usb_bulk_msg(dev->udev, dev->in, rx_buf, len, &alen, 100);
+	ret = usb_bulk_msg(dev->udev, dev->in, rx_buf, len, &alen, 2);
 	if (ret)
 		return ret;
 
+	dev->last_pkt_received = get_time_ns();
+
 	if (alen) {
 		if (info->rx_fixup)
 			return info->rx_fixup(dev, rx_buf, alen);
diff --git a/include/usb/usbnet.h b/include/usb/usbnet.h
index 386c2164bd..ee578b81ed 100644
--- a/include/usb/usbnet.h
+++ b/include/usb/usbnet.h
@@ -52,6 +52,9 @@ struct usbnet {
 #		define EVENT_RX_MEMORY	2
 #		define EVENT_STS_SPLIT	3
 #		define EVENT_LINK_RESET	4
+
+	uint64_t last_pkt_received;
+	uint64_t last_recv_call;
 };
 
 #if 0
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 13/16] net: phy: Also print link down messages

[Sascha Hauer]

Not only print when the link comes up, but also when it goes down.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/phy/phy.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index ccdc9f3716..a440edffd6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -46,6 +46,7 @@ int phy_update_status(struct phy_device *phydev)
 	struct eth_device *edev = phydev->attached_dev;
 	int ret;
 	int oldspeed = phydev->speed, oldduplex = phydev->duplex;
+	int oldlink = phydev->link;
 
 	if (drv) {
 		ret = drv->read_status(phydev);
@@ -53,7 +54,8 @@ int phy_update_status(struct phy_device *phydev)
 			return ret;
 	}
 
-	if (phydev->speed == oldspeed && phydev->duplex == oldduplex)
+	if (phydev->speed == oldspeed && phydev->duplex == oldduplex &&
+	    phydev->link == oldlink)
 		return 0;
 
 	if (phydev->adjust_link)
@@ -62,6 +64,8 @@ int phy_update_status(struct phy_device *phydev)
 	if (phydev->link)
 		dev_info(&edev->dev, "%dMbps %s duplex link detected\n",
 				phydev->speed, phydev->duplex ? "full" : "half");
+	else if (oldlink)
+		dev_info(&edev->dev, "link down\n");
 
 	return 0;
 }
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 14/16] net: ifup command: add ethernet device completion

[Sascha Hauer]

ifup usually takes an ethernet device, so use ethernet device completion
for the command.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/ifup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ifup.c b/net/ifup.c
index e5e8ef2346..fa2c52ff8b 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -21,6 +21,7 @@
 #include <environment.h>
 #include <command.h>
 #include <common.h>
+#include <complete.h>
 #include <getopt.h>
 #include <dhcp.h>
 #include <net.h>
@@ -341,6 +342,7 @@ BAREBOX_CMD_START(ifup)
 	BAREBOX_CMD_DESC("bring a network interface up")
 	BAREBOX_CMD_OPTS("[-af] [INTF]")
 	BAREBOX_CMD_GROUP(CMD_GRP_NET)
+	BAREBOX_CMD_COMPLETE(eth_complete)
 	BAREBOX_CMD_HELP(cmd_ifup_help)
 BAREBOX_CMD_END
 
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 15/16] net: phy: Do not claim the link is up initially

[Sascha Hauer]

A freshly created phy device doesn't have the link up, so drop setting
phydev->link to one initially.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/phy/phy.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index a440edffd6..57c2f8044f 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -165,7 +165,6 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id)
 	phydev->speed = 0;
 	phydev->duplex = -1;
 	phydev->pause = phydev->asym_pause = 0;
-	phydev->link = 1;
 	phydev->autoneg = AUTONEG_ENABLE;
 
 	phydev->addr = addr;
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 16/16] net: Add ifdown support and command

[Sascha Hauer]

ifdown is the counterpart to ifup and disables one or all ethernet
interfaces.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  4 +++
 net/ifup.c    | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/include/net.h b/include/net.h
index 23e3759905..583dc14ed5 100644
--- a/include/net.h
+++ b/include/net.h
@@ -488,6 +488,10 @@ int ifup_edev(struct eth_device *edev, unsigned flags);
 int ifup(const char *name, unsigned flags);
 int ifup_all(unsigned flags);
 
+void ifdown_edev(struct eth_device *edev);
+int ifdown(const char *name);
+void ifdown_all(void);
+
 extern struct list_head netdev_list;
 
 #define for_each_netdev(netdev) list_for_each_entry(netdev, &netdev_list, list)
diff --git a/net/ifup.c b/net/ifup.c
index fa2c52ff8b..b76b4bc44c 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -237,6 +237,12 @@ int ifup_edev(struct eth_device *edev, unsigned flags)
 	return 0;
 }
 
+void ifdown_edev(struct eth_device *edev)
+{
+	eth_close(edev);
+	edev->ifup = false;
+}
+
 int ifup(const char *ethname, unsigned flags)
 {
 	struct eth_device *edev;
@@ -253,6 +259,19 @@ int ifup(const char *ethname, unsigned flags)
 	return ifup_edev(edev, flags);
 }
 
+int ifdown(const char *ethname)
+{
+	struct eth_device *edev;
+
+	edev = eth_get_byname(ethname);
+	if (!edev)
+		return -ENODEV;
+
+	ifdown_edev(edev);
+
+	return 0;
+}
+
 static int net_ifup_force_detect;
 
 int ifup_all(unsigned flags)
@@ -286,6 +305,14 @@ int ifup_all(unsigned flags)
 	return 0;
 }
 
+void ifdown_all(void)
+{
+	struct eth_device *edev;
+
+	for_each_netdev(edev)
+		ifdown_edev(edev);
+}
+
 static int ifup_all_init(void)
 {
 	globalvar_add_simple_bool("net.ifup_force_detect", &net_ifup_force_detect);
@@ -346,4 +373,46 @@ BAREBOX_CMD_START(ifup)
 	BAREBOX_CMD_HELP(cmd_ifup_help)
 BAREBOX_CMD_END
 
+static int do_ifdown(int argc, char *argv[])
+{
+	int opt;
+	int all = 0;
+
+	while ((opt = getopt(argc, argv, "a")) > 0) {
+		switch (opt) {
+		case 'a':
+			all = 1;
+			break;
+		}
+	}
+
+	if (all) {
+		ifdown_all();
+		return 0;
+	}
+
+	if (argc == optind)
+		return COMMAND_ERROR_USAGE;
+
+	return ifdown(argv[optind]);
+}
+
+
+
+BAREBOX_CMD_HELP_START(ifdown)
+BAREBOX_CMD_HELP_TEXT("Disable a network interface")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-a",  "disable all interfaces")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(ifdown)
+	.cmd		= do_ifdown,
+	BAREBOX_CMD_DESC("disable a network interface")
+	BAREBOX_CMD_OPTS("[-a] [INTF]")
+	BAREBOX_CMD_GROUP(CMD_GRP_NET)
+	BAREBOX_CMD_COMPLETE(eth_complete)
+	BAREBOX_CMD_HELP(cmd_ifdown_help)
+BAREBOX_CMD_END
+
 #endif
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox