[PATCH v5 00/21] Slices and fastboot over UDP

[PATCH v5 00/21] Slices and fastboot over UDP

[Sascha Hauer]

I hope we are getting closer to finally get this merged.

Changes since last version:

- may_send now has three states again like in original series
- integrated review feedback from Daniel
- Register/unregister poller when needed
- abort current session when no progress is being made

Daniel Glöckner (2):
  defconfigs: update renamed fastboot options
  fastboot: rename usbgadget.fastboot_* variables to fastboot.*

Edmund Henniges (1):
  fastboot net: implement fastboot over UDP

Sascha Hauer (18):
  Introduce slices
  Add workqueues
  ratp: Switch to workqueues
  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
  globalvar: Add helper for deprecated variable names
  fastboot: Warn when cb_download is called with file still open
  fastboot: Add fastboot_abort()
  fastboot: init list head in common
  usb: fastboot: execute commands in command context
  Add WARN_ONCE() macro
  fs: Warn when filesystem operations are called from a poller
  Documentation: Add document for parallel execution in barebox

 Documentation/devel/background-execution.rst | 123 ++++
 Documentation/devel/devel.rst                |  14 +
 Documentation/index.rst                      |   1 +
 Documentation/user/usb.rst                   |   4 +-
 arch/arm/configs/imx23_defconfig             |   2 +-
 arch/arm/configs/imx28_defconfig             |   2 +-
 arch/arm/configs/imx_v7_defconfig            |   1 +
 arch/arm/configs/imx_v8_defconfig            |   2 +-
 arch/arm/configs/kindle-mx50_defconfig       |   2 +-
 arch/arm/configs/omap_defconfig              |   2 +-
 arch/arm/configs/zii_vf610_dev_defconfig     |   2 +-
 commands/Kconfig                             |   8 +
 commands/usbgadget.c                         |   2 +-
 common/Makefile                              |   2 +
 common/fastboot.c                            |  47 +-
 common/globalvar.c                           |  54 +-
 common/hush.c                                |   6 +
 common/poller.c                              |  11 +-
 common/ratp/ratp.c                           |  57 +-
 common/slice.c                               | 335 +++++++++++
 common/startup.c                             |   3 +
 common/usbgadget.c                           |  16 +-
 common/workqueue.c                           |  58 ++
 drivers/net/phy/mdio_bus.c                   |  43 ++
 drivers/net/usb/usbnet.c                     |  22 +-
 drivers/usb/core/usb.c                       |  12 +-
 drivers/usb/gadget/f_fastboot.c              |  54 +-
 fs/fs.c                                      |  35 ++
 fs/nfs.c                                     |   2 -
 fs/tftp.c                                    |   2 -
 include/asm-generic/bug.h                    |  13 +
 include/fastboot.h                           |   8 +
 include/fastboot_net.h                       |  12 +
 include/globalvar.h                          |   5 +
 include/linux/phy.h                          |  38 +-
 include/net.h                                |  11 +-
 include/ratp_bb.h                            |   1 -
 include/slice.h                              |  42 ++
 include/usb/usb.h                            |   8 +-
 include/usb/usbnet.h                         |   3 +
 include/work.h                               |  29 +
 lib/readline.c                               |   6 +-
 net/Kconfig                                  |  10 +
 net/Makefile                                 |   1 +
 net/dhcp.c                                   |   1 -
 net/dns.c                                    |   1 -
 net/eth.c                                    |  84 ++-
 net/fastboot.c                               | 565 +++++++++++++++++++
 net/net.c                                    |  60 +-
 net/netconsole.c                             |   4 +-
 net/nfs.c                                    |   1 -
 net/ping.c                                   |   2 -
 net/sntp.c                                   |   2 -
 53 files changed, 1716 insertions(+), 115 deletions(-)
 create mode 100644 Documentation/devel/background-execution.rst
 create mode 100644 Documentation/devel/devel.rst
 create mode 100644 common/slice.c
 create mode 100644 common/workqueue.c
 create mode 100644 include/fastboot_net.h
 create mode 100644 include/slice.h
 create mode 100644 include/work.h
 create mode 100644 net/fastboot.c

-- 
2.27.0


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

[PATCH 01/21] 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>
---
 commands/Kconfig |   8 ++
 common/Makefile  |   1 +
 common/slice.c   | 303 +++++++++++++++++++++++++++++++++++++++++++++++
 include/slice.h  |  31 +++++
 4 files changed, 343 insertions(+)
 create mode 100644 common/slice.c
 create mode 100644 include/slice.h

diff --git a/commands/Kconfig b/commands/Kconfig
index 3789f33c3b..608643fceb 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -253,6 +253,14 @@ config CMD_POLLER
 	  is_timeout() or one of the various delay functions. The poller command prints
 	  informations about registered pollers.
 
+config CMD_SLICE
+	tristate
+	prompt "slice"
+	help
+	  slices are a way to protect resources from being accessed by pollers. The slice
+	  command can be used to print informations about slices and also to manipulate
+	  them on the command line for debugging purposes.
+
 # end Information commands
 endmenu
 
diff --git a/common/Makefile b/common/Makefile
index 53859d8d14..141109b23a 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..085d67604f
--- /dev/null
+++ b/common/slice.c
@@ -0,0 +1,303 @@
+// 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_depends_on(&gpio->slice, i2c_device_slice(i2cdev));
+ *
+ * LED driver probe:
+ *
+ * slice_depends_on(&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->name;
+}
+
+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;
+	bool ret = false;
+
+	if (slice->acquired > 0)
+		return true;
+
+	if (slice->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 = 0;
+
+	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_depends_on: 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_depends_on(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_init - initialize a slice before usage
+ * @slice: The slice to initialize
+ * @name:  The name of the slice
+ *
+ * This initializes a slice before usage. @name should be a meaningful name, when
+ * a device context is available to the caller it is recommended to pass its
+ * dev_name() as name argument.
+ */
+void slice_init(struct slice *slice, const char *name)
+{
+	INIT_LIST_HEAD(&slice->deps);
+	slice->name = xstrdup(name);
+	list_add_tail(&slice->list, &slices);
+}
+
+/**
+ * slice_exit: Remove a slice
+ * @slice: The slice to remove the dependency from
+ */
+void slice_exit(struct slice *slice)
+{
+	struct slice *s;
+	struct slice_entry *se, *tmp;
+
+	list_del(&slice->list);
+
+	/* remove our dependencies */
+	list_for_each_entry_safe(se, tmp, &slice->deps, list) {
+		list_del(&se->list);
+		free(se);
+	}
+
+	/* remove this slice from other slices dependencies */
+	list_for_each_entry(s, &slices, list) {
+		list_for_each_entry_safe(se, tmp, &s->deps, list) {
+			if (se->slice == slice) {
+				list_del(&se->list);
+				free(se);
+			}
+		}
+	}
+
+	free(slice->name);
+}
+
+#if defined CONFIG_CMD_SLICE
+
+#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 __slice_acquire_name(const char *name, int add)
+{
+	struct slice *slice;
+
+	list_for_each_entry(slice, &slices, list) {
+		if (!strcmp(slice->name, name)) {
+			__slice_acquire(slice, add);
+			return 0;
+		}
+	}
+
+	printf("No such slice: %s\n", name);
+
+	return 1;
+}
+
+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_END
+
+static int do_slice(int argc, char *argv[])
+{
+	int opt;
+
+	while ((opt = getopt(argc, argv, "a:r:i")) > 0) {
+		switch (opt) {
+		case 'a':
+			return __slice_acquire_name(optarg, 1);
+		case 'r':
+			return __slice_acquire_name(optarg, -1);
+		case 'i':
+			slice_info();
+			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..5538fc434a
--- /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;
+	char *name;
+	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_depends_on(struct slice *slice, struct slice *dep);
+void slice_init(struct slice *slice, const char *name);
+void slice_exit(struct slice *slice);
+
+void slice_debug_acquired(struct slice *slice);
+
+#endif /* __SLICE_H */
-- 
2.27.0


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

[PATCH 02/21] Add workqueues

[Sascha Hauer]

Some code wants to run arbitrary code in the background, examples are
fastboot and ratp. Currently this is implemented in pollers. The problem
with this is that pollers are executed whenever is_timeout() is called
which may happen anywhere in the code. With this we can never tell which
resources are currently in use when the poller is executed.

This adds a work queue interface which is specifically designed to
trigger doing work in a context where it's safe to run arbitrary commands.

Code in pollers can attach work to a work queue which is at that time
only queued up. A new slice, the command slice, is added which by
default is acquired. It is only released at places where the shell waits
for input. When during this time pollers are executed the queued up
works are done before running the registered pollers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/Makefile    |  1 +
 common/hush.c      |  6 +++++
 common/poller.c    |  9 +++++++
 common/slice.c     | 32 +++++++++++++++++++++++++
 common/startup.c   |  3 +++
 common/workqueue.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 include/slice.h    |  5 ++++
 include/work.h     | 29 +++++++++++++++++++++++
 8 files changed, 143 insertions(+)
 create mode 100644 common/workqueue.c
 create mode 100644 include/work.h

diff --git a/common/Makefile b/common/Makefile
index 141109b23a..603edd4f52 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_ELF)		+= elf.o
 obj-y				+= restart.o
 obj-y				+= poweroff.o
 obj-y				+= slice.o
+obj-y				+= workqueue.o
 obj-$(CONFIG_MACHINE_ID)	+= machine_id.o
 obj-$(CONFIG_AUTO_COMPLETE)	+= complete.o
 obj-y				+= version.o
diff --git a/common/hush.c b/common/hush.c
index c24b2c7cd2..ec0d5129b7 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -121,6 +121,7 @@
 #include <libbb.h>
 #include <password.h>
 #include <glob.h>
+#include <slice.h>
 #include <getopt.h>
 #include <libfile.h>
 #include <libbb.h>
@@ -460,7 +461,12 @@ static void get_user_input(struct in_str *i)
 	else
 		prompt = CONFIG_PROMPT_HUSH_PS2;
 
+	command_slice_release();
+
 	n = readline(prompt, console_buffer, CONFIG_CBSIZE);
+
+	command_slice_acquire();
+
 	if (n == -1 ) {
 		i->interrupt = 1;
 		n = 0;
diff --git a/common/poller.c b/common/poller.c
index 95f828b439..7b1b92714c 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -12,6 +12,8 @@
 #include <param.h>
 #include <poller.h>
 #include <clock.h>
+#include <work.h>
+#include <slice.h>
 
 static LIST_HEAD(poller_list);
 static int poller_active;
@@ -110,15 +112,22 @@ int poller_async_unregister(struct poller_async *pa)
 void poller_call(void)
 {
 	struct poller_struct *poller, *tmp;
+	bool run_workqueues = !slice_acquired(&command_slice);
 
 	if (poller_active)
 		return;
 
+	command_slice_acquire();
+
+	if (run_workqueues)
+		wq_do_all_works();
+
 	poller_active = 1;
 
 	list_for_each_entry_safe(poller, tmp, &poller_list, list)
 		poller->func(poller);
 
+	command_slice_release();
 	poller_active = 0;
 }
 
diff --git a/common/slice.c b/common/slice.c
index 085d67604f..62e5d56d89 100644
--- a/common/slice.c
+++ b/common/slice.c
@@ -3,6 +3,7 @@
 #define pr_fmt(fmt) "slice: " fmt
 
 #include <common.h>
+#include <init.h>
 #include <slice.h>
 
 /*
@@ -231,6 +232,37 @@ void slice_exit(struct slice *slice)
 	free(slice->name);
 }
 
+struct slice command_slice;
+
+/**
+ * command_slice_acquire - acquire the command slice
+ *
+ * The command slice is acquired by default. It is only released
+ * at certain points we know it's safe to execute code in the
+ * background, like when the shell is waiting for input.
+ */
+void command_slice_acquire(void)
+{
+	slice_acquire(&command_slice);
+}
+
+/**
+ * command_slice_release - release the command slice
+ */
+void command_slice_release(void)
+{
+	slice_release(&command_slice);
+}
+
+static int command_slice_init(void)
+{
+	slice_init(&command_slice, "idle");
+	slice_acquire(&command_slice);
+	return 0;
+}
+
+pure_initcall(command_slice_init);
+
 #if defined CONFIG_CMD_SLICE
 
 #include <command.h>
diff --git a/common/startup.c b/common/startup.c
index 511675ed55..7425e31882 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -34,6 +34,7 @@
 #include <debug_ll.h>
 #include <fs.h>
 #include <errno.h>
+#include <slice.h>
 #include <linux/stat.h>
 #include <envfs.h>
 #include <magicvar.h>
@@ -268,8 +269,10 @@ enum autoboot_state do_autoboot_countdown(void)
 		break;
 	}
 
+	command_slice_release();
 	ret = console_countdown(global_autoboot_timeout, flags, abortkeys,
 				&outkey);
+	command_slice_acquire();
 
 	if (ret == 0)
 		autoboot_state = AUTOBOOT_BOOT;
diff --git a/common/workqueue.c b/common/workqueue.c
new file mode 100644
index 0000000000..6fdd4e42ea
--- /dev/null
+++ b/common/workqueue.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <common.h>
+#include <work.h>
+
+static void wq_do_pending_work(struct work_queue *wq)
+{
+	struct work_struct *work, *tmp;
+
+	list_for_each_entry_safe(work, tmp, &wq->work, list) {
+		list_del(&work->list);
+		wq->fn(work);
+	}
+}
+
+void wq_cancel_work(struct work_queue *wq)
+{
+	struct work_struct *work, *tmp;
+
+	list_for_each_entry_safe(work, tmp, &wq->work, list) {
+		list_del(&work->list);
+		wq->cancel(work);
+	}
+}
+
+static LIST_HEAD(work_queues);
+
+/**
+ * wq_do_all_works - do all pending work
+ *
+ * This calls all pending work functions
+ */
+void wq_do_all_works(void)
+{
+	struct work_queue *wq;
+
+	list_for_each_entry(wq, &work_queues, list)
+		wq_do_pending_work(wq);
+}
+
+/**
+ * wq_register - register a new work queue
+ * @wq:    The work queue
+ */
+void wq_register(struct work_queue *wq)
+{
+	INIT_LIST_HEAD(&wq->work);
+	list_add_tail(&wq->list, &work_queues);
+}
+
+/**
+ * wq_unregister - unregister a work queue
+ * @wq:    The work queue
+ */
+void wq_unregister(struct work_queue *wq)
+{
+	wq_cancel_work(wq);
+	list_del(&wq->list);
+}
diff --git a/include/slice.h b/include/slice.h
index 5538fc434a..fd753e194b 100644
--- a/include/slice.h
+++ b/include/slice.h
@@ -28,4 +28,9 @@ void slice_exit(struct slice *slice);
 
 void slice_debug_acquired(struct slice *slice);
 
+extern struct slice command_slice;
+
+void command_slice_acquire(void);
+void command_slice_release(void);
+
 #endif /* __SLICE_H */
diff --git a/include/work.h b/include/work.h
new file mode 100644
index 0000000000..e428e821ea
--- /dev/null
+++ b/include/work.h
@@ -0,0 +1,29 @@
+#ifndef __WORK_H
+#define __WORK_H
+
+#include <linux/list.h>
+
+struct work_struct {
+	struct list_head list;
+};
+
+struct work_queue {
+	void (*fn)(struct work_struct *work);
+	void (*cancel)(struct work_struct *work);
+
+	struct list_head list;
+	struct list_head work;
+};
+
+static inline void wq_queue_work(struct work_queue *wq, struct work_struct *work)
+{
+	list_add_tail(&work->list, &wq->work);
+}
+
+void wq_register(struct work_queue *wq);
+void wq_unregister(struct work_queue *wq);
+
+void wq_do_all_works(void);
+void wq_cancel_work(struct work_queue *wq);
+
+#endif /* __WORK_H */
-- 
2.27.0


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

[PATCH 03/21] ratp: Switch to workqueues

[Sascha Hauer]

This switches running barebox commands in ratp to a context where it's
safe to do so: In a work queue.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/ratp/ratp.c | 57 ++++++++++++++++++++++++++++++++++------------
 include/ratp_bb.h  |  1 -
 lib/readline.c     |  6 +----
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index b8043fe5c7..84910cacb7 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -24,6 +24,7 @@
 #include <environment.h>
 #include <kfifo.h>
 #include <poller.h>
+#include <work.h>
 #include <linux/sizes.h>
 #include <ratp_bb.h>
 #include <fs.h>
@@ -50,9 +51,11 @@ struct ratp_ctx {
 	struct ratp_bb_pkt *fs_rx;
 
 	struct poller_struct poller;
+	struct work_queue wq;
 
 	bool console_registered;
 	bool poller_registered;
+	bool wq_registered;
 };
 
 static int compare_ratp_command(struct list_head *a, struct list_head *b)
@@ -194,9 +197,14 @@ static int ratp_bb_send_command_return(struct ratp_ctx *ctx, uint32_t errno)
 	return ret;
 }
 
-static char *ratp_command;
 static struct ratp_ctx *ratp_ctx;
 
+struct ratp_work {
+	struct work_struct work;
+	struct ratp_ctx *ctx;
+	char *command;
+};
+
 static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 {
 	const struct ratp_bb *rbb = buf;
@@ -205,6 +213,7 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 	int ret = 0;
 	uint16_t type = be16_to_cpu(rbb->type);
 	struct ratp_command *cmd;
+	struct ratp_work *rw;
 
 	/* See if there's a command registered to this type */
 	cmd = find_ratp_request(type);
@@ -222,12 +231,16 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 
 	switch (type) {
 	case BB_RATP_TYPE_COMMAND:
-		if (!IS_ENABLED(CONFIG_CONSOLE_RATP) || ratp_command)
+		if (!IS_ENABLED(CONFIG_CONSOLE_RATP))
 			return 0;
 
-		ratp_command = xstrndup((const char *)rbb->data, dlen);
-		ratp_ctx = ctx;
-		pr_debug("got command: %s\n", ratp_command);
+		rw = xzalloc(sizeof(*rw));
+		rw->ctx = ctx;
+		rw->command = xstrndup((const char *)rbb->data, dlen);
+
+		wq_queue_work(&ctx->wq, &rw->work);
+
+		pr_debug("got command: %s\n", rw->command);
 
 		break;
 
@@ -298,21 +311,20 @@ static void ratp_console_putc(struct console_device *cdev, char c)
 	kfifo_putc(ctx->console_transmit_fifo, c);
 }
 
-void barebox_ratp_command_run(void)
+static void ratp_command_run(struct work_struct *w)
 {
+	struct ratp_work *rw = container_of(w, struct ratp_work, work);
+	struct ratp_ctx *ctx = rw->ctx;
 	int ret;
 
-	if (!ratp_command)
-		return;
-
-	pr_debug("running command: %s\n", ratp_command);
+	pr_debug("running command: %s\n", rw->command);
 
-	ret = run_command(ratp_command);
+	ret = run_command(rw->command);
 
-	free(ratp_command);
-	ratp_command = NULL;
+	free(rw->command);
+	free(rw);
 
-	ratp_bb_send_command_return(ratp_ctx, ret);
+	ratp_bb_send_command_return(ctx, ret);
 }
 
 static const char *ratpfs_mount_path;
@@ -337,6 +349,9 @@ static void ratp_unregister(struct ratp_ctx *ctx)
 	if (ctx->poller_registered)
 		poller_unregister(&ctx->poller);
 
+	if (ctx->wq_registered)
+		wq_unregister(&ctx->wq);
+
 	ratp_close(&ctx->ratp);
 	console_set_active(ctx->cdev, ctx->old_active);
 
@@ -369,6 +384,7 @@ static void ratp_poller(struct poller_struct *poller)
 	ret = ratp_poll(&ctx->ratp);
 	if (ret == -EINTR)
 		goto out;
+
 	if (ratp_closed(&ctx->ratp))
 		goto out;
 
@@ -424,6 +440,14 @@ int barebox_ratp_fs_call(struct ratp_bb_pkt *tx, struct ratp_bb_pkt **rx)
 	return 0;
 }
 
+static void ratp_work_cancel(struct work_struct *w)
+{
+	struct ratp_work *rw = container_of(w, struct ratp_work, work);
+
+	free(rw->command);
+	free(rw);
+}
+
 int barebox_ratp(struct console_device *cdev)
 {
 	int ret;
@@ -467,6 +491,11 @@ int barebox_ratp(struct console_device *cdev)
 	if (ret < 0)
 		goto out;
 
+	ctx->wq.fn = ratp_command_run;
+	ctx->wq.cancel = ratp_work_cancel;
+	wq_register(&ctx->wq);
+	ctx->wq_registered = true;
+
 	ret = poller_register(&ctx->poller, "ratp");
 	if (ret)
 		goto out;
diff --git a/include/ratp_bb.h b/include/ratp_bb.h
index a4f28c642c..b710f99bf9 100644
--- a/include/ratp_bb.h
+++ b/include/ratp_bb.h
@@ -41,7 +41,6 @@ struct ratp_bb_pkt {
 };
 
 int  barebox_ratp(struct console_device *cdev);
-void barebox_ratp_command_run(void);
 int  barebox_ratp_fs_call(struct ratp_bb_pkt *tx, struct ratp_bb_pkt **rx);
 int  barebox_ratp_fs_mount(const char *path);
 
diff --git a/lib/readline.c b/lib/readline.c
index 3d16c1838c..e5370f9c7b 100644
--- a/lib/readline.c
+++ b/lib/readline.c
@@ -3,7 +3,6 @@
 #include <init.h>
 #include <libbb.h>
 #include <poller.h>
-#include <ratp_bb.h>
 #include <xfuncs.h>
 #include <complete.h>
 #include <linux/ctype.h>
@@ -200,11 +199,8 @@ int readline(const char *prompt, char *buf, int len)
 	puts (prompt);
 
 	while (1) {
-		while (!tstc()) {
+		while (!tstc())
 			poller_call();
-			if (IS_ENABLED(CONFIG_CONSOLE_RATP))
-				barebox_ratp_command_run();
-		}
 
 		ichar = read_key();
 
-- 
2.27.0


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

[PATCH 04/21] 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     | 75 ++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/include/net.h b/include/net.h
index 54db8a179a..8d2b4923de 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 e3d0d06efe..a51e3421a7 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -22,6 +22,7 @@
 #include <init.h>
 #include <dhcp.h>
 #include <net.h>
+#include <dma.h>
 #include <of.h>
 #include <linux/phy.h>
 #include <errno.h>
@@ -208,10 +209,61 @@ static int eth_carrier_check(struct eth_device *edev, int force)
 	return edev->phydev->link ? 0 : -ENETDOWN;
 }
 
+struct eth_q {
+	struct eth_device *edev;
+	int length;
+	struct list_head list;
+	void *data;
+};
+
+static int eth_queue(struct list_head *list, struct eth_device *edev,
+		     void *packet, int length)
+{
+	struct eth_q *q;
+
+	q = xzalloc(sizeof(*q));
+	if (!q)
+		return -ENOMEM;
+
+	q->data = dma_alloc(length);
+	if (!q->data) {
+		free(q);
+		return -ENOMEM;
+	}
+
+	q->length = length;
+	q->edev = edev;
+
+	memcpy(q->data, packet, length);
+	list_add_tail(&q->list, list);
+
+	return 0;
+}
+
+static LIST_HEAD(eth_send_list);
+
+static void eth_send_queue(void)
+{
+	struct eth_q *q, *tmp;
+
+	list_for_each_entry_safe(q, tmp, &eth_send_list, list) {
+		if (slice_acquired(eth_device_slice(q->edev)))
+			continue;
+
+		eth_send(q->edev, q->data, q->length);
+		list_del(&q->list);
+		free(q->data);
+		free(q);
+	}
+}
+
 int eth_send(struct eth_device *edev, void *packet, int length)
 {
 	int ret;
 
+	if (slice_acquired(eth_device_slice(edev)))
+		return eth_queue(&eth_send_list, edev, packet, length);
+
 	if (!edev->active)
 		return -ENETDOWN;
 
@@ -219,20 +271,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;
 
-	return edev->recv(edev);
+	ret = edev->recv(edev);
+out:
+	slice_release(eth_device_slice(edev));
+
+	return ret;
 }
 
 int eth_rx(void)
@@ -244,6 +308,8 @@ int eth_rx(void)
 			__eth_rx(edev);
 	}
 
+	eth_send_queue();
+
 	return 0;
 }
 
@@ -353,6 +419,8 @@ int eth_register(struct eth_device *edev)
 	if (ret)
 		return ret;
 
+	slice_init(&edev->slice, dev_name(dev));
+
 	edev->devname = xstrdup(dev_name(&edev->dev));
 
 	dev_add_param_ip(dev, "ipaddr", NULL, NULL, &edev->ipaddr, edev);
@@ -431,6 +499,7 @@ void eth_unregister(struct eth_device *edev)
 	free(edev->devname);
 
 	unregister_device(&edev->dev);
+	slice_exit(&edev->slice);
 	list_del(&edev->list);
 }
 
-- 
2.27.0


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

[PATCH 05/21] 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 | 43 ++++++++++++++++++++++++++++++++++++++
 include/linux/phy.h        | 38 +++++++++++++--------------------
 2 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 3480e2ffb4..99d23ffedf 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -239,6 +239,8 @@ int mdiobus_register(struct mii_bus *bus)
 		return -EINVAL;
 	}
 
+	slice_init(&bus->slice, dev_name(&bus->dev));
+
 	if (bus->reset)
 		bus->reset(bus);
 
@@ -272,6 +274,8 @@ void mdiobus_unregister(struct mii_bus *bus)
 		bus->phy_map[i] = NULL;
 	}
 
+	slice_exit(&bus->slice);
+
 	list_del(&bus->list);
 }
 EXPORT_SYMBOL(mdiobus_unregister);
@@ -357,6 +361,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 eec1332c9d..cdcb7c24f2 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
  *
@@ -397,11 +385,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 && phydev->bus && 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.27.0


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

[PATCH 06/21] usb: Add a slice to usb host controllers

[Sascha Hauer]

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

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 30c251f405..c068c64c6b 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -75,28 +75,30 @@ static int host_busnum = 1;
 
 static inline int usb_host_acquire(struct usb_host *host)
 {
-	if (host->sem)
+	if (slice_acquired(&host->slice))
 		return -EAGAIN;
-	host->sem++;
+
+	slice_acquire(&host->slice);
+
 	return 0;
 }
 
 static inline void usb_host_release(struct usb_host *host)
 {
-	if (host->sem > 0)
-		host->sem--;
+	slice_release(&host->slice);
 }
 
 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, dev_name(host->hw_dev));
 	return 0;
 }
 
 void usb_unregister_host(struct usb_host *host)
 {
+	slice_exit(&host->slice);
 	list_del(&host->list);
 }
 
diff --git a/include/usb/usb.h b/include/usb/usb.h
index c2085eae87..39f4750916 100644
--- a/include/usb/usb.h
+++ b/include/usb/usb.h
@@ -20,6 +20,7 @@
 #define _USB_H_
 
 #include <driver.h>
+#include <slice.h>
 #include <usb/ch9.h>
 #include <usb/ch11.h>
 #include <usb/usb_defs.h>
@@ -163,13 +164,18 @@ struct usb_host {
 	struct device_d *hw_dev;
 	int busnum;
 	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.27.0


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

[PATCH 07/21] 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 7397034586..46ea73247f 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -233,6 +233,9 @@ int usbnet_probe(struct usb_device *usbdev, const struct usb_device_id *prod)
 
 	eth_register(edev);
 
+	slice_depends_on(eth_device_slice(edev), usb_device_slice(usbdev));
+	slice_depends_on(mdiobus_slice(&undev->miibus), usb_device_slice(usbdev));
+
 	return 0;
 out1:
 	dev_dbg(&edev->dev, "err: %d\n", status);
-- 
2.27.0


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

[PATCH 08/21] 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 6c4637281d..6844d13d50 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -447,8 +447,6 @@ again:
 	nfs_timer_start = get_time_ns();
 
 	while (1) {
-		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 d186e7983a..e5a58a4dae 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -207,8 +207,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 8d2b4923de..583dc14ed5 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 a51e3421a7..d53d60d49c 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -286,14 +286,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 5fd6246ab2..197b551e72 100644
--- a/net/net.c
+++ b/net/net.c
@@ -239,8 +239,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",
@@ -249,11 +247,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, "net");
+}
+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 0fece65a23..c692db942f 100644
--- a/net/netconsole.c
+++ b/net/netconsole.c
@@ -59,7 +59,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);
 
@@ -77,8 +77,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 591417e0de..266351da09 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -710,7 +710,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.27.0


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

[PATCH 09/21] 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 197b551e72..cb0013519c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -570,12 +570,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);
@@ -612,7 +654,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.27.0


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

[PATCH 10/21] 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 46ea73247f..aa48678f87 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -122,12 +122,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, dev->rx_buf, len, &alen, 100);
+	ret = usb_bulk_msg(dev->udev, dev->in, dev->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, dev->rx_buf, alen);
diff --git a/include/usb/usbnet.h b/include/usb/usbnet.h
index 450db47b40..7ff32f280a 100644
--- a/include/usb/usbnet.h
+++ b/include/usb/usbnet.h
@@ -54,6 +54,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.27.0


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

[PATCH 11/21] defconfigs: update renamed fastboot options

[Sascha Hauer]

From: Daniel Glöckner <dg@emlix.com>

The split of the generic Fastboot code from the Fastboot USB gadget
included renaming the CONFIG_USB_GADGET_FASTBOOT_* options to
CONFIG_FASTBOOT_*. Update all defconfigs to use the new names.

Signed-off-by: Daniel Glöckner <dg@emlix.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/configs/imx23_defconfig         | 2 +-
 arch/arm/configs/imx28_defconfig         | 2 +-
 arch/arm/configs/imx_v7_defconfig        | 1 +
 arch/arm/configs/imx_v8_defconfig        | 2 +-
 arch/arm/configs/kindle-mx50_defconfig   | 2 +-
 arch/arm/configs/omap_defconfig          | 2 +-
 arch/arm/configs/zii_vf610_dev_defconfig | 2 +-
 7 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm/configs/imx23_defconfig b/arch/arm/configs/imx23_defconfig
index bff9c08c40..48bf14a390 100644
--- a/arch/arm/configs/imx23_defconfig
+++ b/arch/arm/configs/imx23_defconfig
@@ -22,6 +22,7 @@ CONFIG_BLSPEC=y
 CONFIG_CONSOLE_ALLOW_COLOR=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_RESET_SOURCE=y
+CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_IOMEM=y
@@ -90,7 +91,6 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_DFU=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
-CONFIG_USB_GADGET_FASTBOOT_CMD_OEM=y
 CONFIG_VIDEO=y
 CONFIG_DRIVER_VIDEO_STM=y
 CONFIG_MCI=y
diff --git a/arch/arm/configs/imx28_defconfig b/arch/arm/configs/imx28_defconfig
index 4442c79cc4..beb0bc2f76 100644
--- a/arch/arm/configs/imx28_defconfig
+++ b/arch/arm/configs/imx28_defconfig
@@ -25,6 +25,7 @@ CONFIG_CONSOLE_ALLOW_COLOR=y
 CONFIG_PBL_CONSOLE=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_RESET_SOURCE=y
+CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_IOMEM=y
@@ -105,7 +106,6 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_DFU=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
-CONFIG_USB_GADGET_FASTBOOT_CMD_OEM=y
 CONFIG_VIDEO=y
 CONFIG_DRIVER_VIDEO_STM=y
 CONFIG_MCI=y
diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
index 4fa8b19a6b..b1fe71786f 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -70,6 +70,7 @@ CONFIG_CONSOLE_ACTIVATE_NONE=y
 CONFIG_PARTITION_DISK_EFI=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_RESET_SOURCE=y
+CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_IOMEM=y
diff --git a/arch/arm/configs/imx_v8_defconfig b/arch/arm/configs/imx_v8_defconfig
index 06fb406084..b96867ad0c 100644
--- a/arch/arm/configs/imx_v8_defconfig
+++ b/arch/arm/configs/imx_v8_defconfig
@@ -25,6 +25,7 @@ CONFIG_CONSOLE_RATP=y
 CONFIG_PARTITION_DISK_EFI=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_RESET_SOURCE=y
+CONFIG_FASTBOOT_SPARSE=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_IOMEM=y
@@ -106,7 +107,6 @@ CONFIG_USB_STORAGE=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
-CONFIG_USB_GADGET_FASTBOOT_SPARSE=y
 CONFIG_MCI=y
 CONFIG_MCI_MMC_BOOT_PARTITIONS=y
 CONFIG_MCI_IMX_ESDHC=y
diff --git a/arch/arm/configs/kindle-mx50_defconfig b/arch/arm/configs/kindle-mx50_defconfig
index 855daef71a..552b2d6d33 100644
--- a/arch/arm/configs/kindle-mx50_defconfig
+++ b/arch/arm/configs/kindle-mx50_defconfig
@@ -20,6 +20,7 @@ CONFIG_BOOTM_OFTREE=y
 CONFIG_CONSOLE_ACTIVATE_ALL=y
 CONFIG_CONSOLE_ALLOW_COLOR=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
+CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_MEMINFO=y
@@ -50,7 +51,6 @@ CONFIG_USB_EHCI=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
-CONFIG_USB_GADGET_FASTBOOT_CMD_OEM=y
 CONFIG_MCI=y
 CONFIG_MCI_STARTUP=y
 CONFIG_MCI_MMC_BOOT_PARTITIONS=y
diff --git a/arch/arm/configs/omap_defconfig b/arch/arm/configs/omap_defconfig
index 9d71d02744..59892cb231 100644
--- a/arch/arm/configs/omap_defconfig
+++ b/arch/arm/configs/omap_defconfig
@@ -34,6 +34,7 @@ CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_STATE=y
 CONFIG_BOOTCHOOSER=y
 CONFIG_RESET_SOURCE=y
+CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_IOMEM=y
 CONFIG_CMD_MEMINFO=y
@@ -121,7 +122,6 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_DFU=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
-CONFIG_USB_GADGET_FASTBOOT_CMD_OEM=y
 CONFIG_USB_MUSB=y
 CONFIG_USB_MUSB_AM335X=y
 CONFIG_USB_MUSB_HOST=y
diff --git a/arch/arm/configs/zii_vf610_dev_defconfig b/arch/arm/configs/zii_vf610_dev_defconfig
index 7161d740ac..45c24d6df4 100644
--- a/arch/arm/configs/zii_vf610_dev_defconfig
+++ b/arch/arm/configs/zii_vf610_dev_defconfig
@@ -22,6 +22,7 @@ CONFIG_CONSOLE_RATP=y
 CONFIG_PARTITION_DISK_EFI=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_RESET_SOURCE=y
+CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
 CONFIG_CMD_IOMEM=y
@@ -113,7 +114,6 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_DFU=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
-CONFIG_USB_GADGET_FASTBOOT_CMD_OEM=y
 CONFIG_MCI=y
 CONFIG_MCI_MMC_BOOT_PARTITIONS=y
 CONFIG_MCI_IMX_ESDHC=y
-- 
2.27.0


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

[PATCH 12/21] globalvar: Add helper for deprecated variable names

[Sascha Hauer]

When globalvars are renamed across releases it's not nice when variables
in the persistent environment loose their meaning. This adds a helper
function which adds an alias for the old names. When the persistent
variables still use the old names then their values are automatically
written to variables with the new names.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/globalvar.c  | 54 +++++++++++++++++++++++++++++++++++++++++++--
 include/globalvar.h |  5 +++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/common/globalvar.c b/common/globalvar.c
index c87f2c9339..51d93ad385 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -293,6 +293,53 @@ int nvvar_remove(const char *name)
 	return ret;
 }
 
+struct globalvar_deprecated {
+	char *newname;
+	char *oldname;
+	struct list_head list;
+};
+
+static LIST_HEAD(globalvar_deprecated_list);
+
+/*
+ * globalvar_alias_deprecated - add an alias
+ *
+ * @oldname: The old name for the variable
+ * @newname: The new name for the variable
+ *
+ * This function is a helper for globalvars that are renamed from one
+ * release to another. when a variable @oldname is found in the persistent
+ * environment a warning is issued and its value is written to @newname.
+ *
+ * Note that when both @oldname and @newname contain values then the values
+ * existing in @newname are overwritten.
+ */
+void globalvar_alias_deprecated(const char *oldname, const char *newname)
+{
+	struct globalvar_deprecated *gd;
+
+	gd = xzalloc(sizeof(*gd));
+	gd->newname = xstrdup(newname);
+	gd->oldname = xstrdup(oldname);
+	list_add_tail(&gd->list, &globalvar_deprecated_list);
+}
+
+static const char *globalvar_new_name(const char *oldname)
+{
+	struct globalvar_deprecated *gd;
+
+	list_for_each_entry(gd, &globalvar_deprecated_list, list) {
+		if (!strcmp(oldname, gd->oldname)) {
+			pr_warn("nv.%s is deprecated, converting to nv.%s\n", oldname,
+				gd->newname);
+			nv_dirty = 1;
+			return gd->newname;
+		}
+	}
+
+	return oldname;
+}
+
 int nvvar_load(void)
 {
 	char *val;
@@ -308,6 +355,8 @@ int nvvar_load(void)
 		return -ENOENT;
 
 	while ((d = readdir(dir))) {
+		const char *n;
+
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;
 
@@ -316,10 +365,11 @@ int nvvar_load(void)
 		pr_debug("%s: Setting \"%s\" to \"%s\"\n",
 				__func__, d->d_name, val);
 
-		ret = __nvvar_add(d->d_name, val);
+		n = globalvar_new_name(d->d_name);
+		ret = __nvvar_add(n, val);
 		if (ret)
 			pr_err("failed to create nv variable %s: %s\n",
-					d->d_name, strerror(-ret));
+					n, strerror(-ret));
 	}
 
 	closedir(dir);
diff --git a/include/globalvar.h b/include/globalvar.h
index fc85e93e14..db229e239c 100644
--- a/include/globalvar.h
+++ b/include/globalvar.h
@@ -33,6 +33,7 @@ int nvvar_remove(const char *name);
 void globalvar_print(void);
 
 void dev_param_init_from_nv(struct device_d *dev, const char *name);
+void globalvar_alias_deprecated(const char *newname, const char *oldname);
 
 #else
 static inline int globalvar_add_simple(const char *name, const char *value)
@@ -114,6 +115,10 @@ static inline void dev_param_init_from_nv(struct device_d *dev, const char *name
 {
 }
 
+static inline void globalvar_alias_deprecated(const char *newname, const char *oldname)
+{
+}
+
 #endif
 
 void nv_var_set_clean(void);
-- 
2.27.0


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

[PATCH 13/21] fastboot: rename usbgadget.fastboot_* variables to fastboot.*

[Sascha Hauer]

From: Daniel Glöckner <dg@emlix.com>

There is nothing USB-specific in the defined usbgadget.fastboot_*
variables. Rename them to be usable also for the UDP fastboot transport.

The usbgadget.fastboot_function variable is used to define the files and
devices accessible with the erase and flash commands. Since "function" is
a term from the USB specification and the Fastboot specification uses the
term "partition", we rename that variable to "fastboot.partitions".

Signed-off-by: Daniel Glöckner <dg@emlix.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/usb.rst |  4 ++--
 commands/usbgadget.c       |  2 +-
 common/fastboot.c          | 24 +++++++++++++++++++++---
 common/usbgadget.c         | 16 ++--------------
 include/fastboot.h         |  3 +++
 5 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/Documentation/user/usb.rst b/Documentation/user/usb.rst
index 4c1b2925f2..ca5f8138de 100644
--- a/Documentation/user/usb.rst
+++ b/Documentation/user/usb.rst
@@ -266,7 +266,7 @@ USB Gadget autostart Options
   See :ref:`command_usbgadget` -a. (Default 0).
 ``global.usbgadget.dfu_function``
   Function description for DFU. See :ref:`command_usbgadget` -D [desc].
-``global.usbgadget.fastboot_function``
+``global.fastboot.partitions``
   Function description for fastboot. See :ref:`command_usbgadget` -A [desc].
-``global.usbgadget.fastboot_bbu``
+``global.fastboot.bbu``
   Export barebox update handlers. See :ref:`command_usbgadget` -b. (Default 0).
diff --git a/commands/usbgadget.c b/commands/usbgadget.c
index 193da86dee..9133402f52 100644
--- a/commands/usbgadget.c
+++ b/commands/usbgadget.c
@@ -57,7 +57,7 @@ BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT ("-a\t", "Create CDC ACM function")
 BAREBOX_CMD_HELP_OPT ("-A <desc>", "Create Android Fastboot function. If 'desc' is not provided, "
-				   "try to use 'global.usbgadget.fastboot_function' variable.")
+				   "try to use 'global.fastboot.partitions' variable.")
 BAREBOX_CMD_HELP_OPT ("-b\t", "include registered barebox update handlers (fastboot specific)")
 BAREBOX_CMD_HELP_OPT ("-D <desc>", "Create DFU function. If 'desc' is not provided, "
 				   "try to use 'global.usbgadget.dfu_function' variable.")
diff --git a/common/fastboot.c b/common/fastboot.c
index 302720c43d..c800e89a65 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -46,6 +46,8 @@
 #define FASTBOOT_VERSION		"0.4"
 
 static unsigned int fastboot_max_download_size = SZ_8M;
+int fastboot_bbu;
+char *fastboot_partitions;
 
 struct fb_variable {
 	char *name;
@@ -906,14 +908,30 @@ void fastboot_exec_cmd(struct fastboot *fb, const char *cmdbuf)
 static int fastboot_globalvars_init(void)
 {
 	if (IS_ENABLED(CONFIG_FASTBOOT_SPARSE))
-		globalvar_add_simple_int("usbgadget.fastboot_max_download_size",
+		globalvar_add_simple_int("fastboot.max_download_size",
 				 &fastboot_max_download_size, "%u");
+	globalvar_add_simple_bool("fastboot.bbu", &fastboot_bbu);
+	globalvar_add_simple_string("fastboot.partitions",
+				    &fastboot_partitions);
+
+	globalvar_alias_deprecated("usbgadget.fastboot_function",
+				   "fastboot.partitions");
+	globalvar_alias_deprecated("usbgadget.fastboot_bbu",
+				   "fastboot.bbu");
+	globalvar_alias_deprecated("usbgadget.fastboot_max_download_size",
+				   "fastboot.max_download_size");
 
 	return 0;
 }
 
 device_initcall(fastboot_globalvars_init);
 
-BAREBOX_MAGICVAR_NAMED(global_usbgadget_fastboot_max_download_size,
-		       global.usbgadget.fastboot_max_download_size,
+BAREBOX_MAGICVAR_NAMED(global_fastboot_max_download_size,
+		       global.fastboot.max_download_size,
 		       "Fastboot maximum download size");
+BAREBOX_MAGICVAR_NAMED(global_fastboot_partitions,
+		       global.fastboot.partitions,
+		       "Partitions exported for update via fastboot");
+BAREBOX_MAGICVAR_NAMED(global_fastboot_bbu,
+		       global.fastboot.bbu,
+		       "Export barebox update handlers via fastboot");
diff --git a/common/usbgadget.c b/common/usbgadget.c
index a8f104cf1c..25fd994930 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -30,8 +30,6 @@
 static int autostart;
 static int acm;
 static char *dfu_function;
-static char *fastboot_function;
-static int fastboot_bbu;
 
 static struct file_list *parse(const char *files)
 {
@@ -56,8 +54,8 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 		dfu_opts = dfu_function;
 
 	if (fastboot && !fastboot_opts &&
-	    fastboot_function && *fastboot_function)
-		fastboot_opts = fastboot_function;
+	    fastboot_partitions && *fastboot_partitions)
+		fastboot_opts = fastboot_partitions;
 
 	if (!dfu_opts && !fastboot_opts && !acm)
 		return COMMAND_ERROR_USAGE;
@@ -116,12 +114,8 @@ static int usbgadget_globalvars_init(void)
 	if (IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART)) {
 		globalvar_add_simple_bool("usbgadget.autostart", &autostart);
 		globalvar_add_simple_bool("usbgadget.acm", &acm);
-		globalvar_add_simple_bool("usbgadget.fastboot_bbu",
-					  &fastboot_bbu);
 	}
 	globalvar_add_simple_string("usbgadget.dfu_function", &dfu_function);
-	globalvar_add_simple_string("usbgadget.fastboot_function",
-				    &fastboot_function);
 
 	return 0;
 }
@@ -136,9 +130,3 @@ BAREBOX_MAGICVAR_NAMED(global_usbgadget_acm,
 BAREBOX_MAGICVAR_NAMED(global_usbgadget_dfu_function,
 		       global.usbgadget.dfu_function,
 		       "usbgadget: Create DFU function");
-BAREBOX_MAGICVAR_NAMED(global_usbgadget_fastboot_function,
-		       global.usbgadget.fastboot_function,
-		       "usbgadget: Create Android Fastboot function");
-BAREBOX_MAGICVAR_NAMED(global_usbgadget_fastboot_bbu,
-		       global.usbgadget.fastboot_bbu,
-		       "usbgadget: export barebox update handlers via fastboot");
diff --git a/include/fastboot.h b/include/fastboot.h
index d33f9d1851..8619dcd34e 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -53,6 +53,9 @@ enum fastboot_msg_type {
 	FASTBOOT_MSG_DATA,
 };
 
+extern int fastboot_bbu;
+extern char *fastboot_partitions;
+
 int fastboot_generic_init(struct fastboot *fb, bool export_bbu);
 void fastboot_generic_close(struct fastboot *fb);
 void fastboot_generic_free(struct fastboot *fb);
-- 
2.27.0


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

[PATCH 14/21] fastboot: Warn when cb_download is called with file still open

[Sascha Hauer]

Warn when we are about to open a new download file without having
closed the old one.

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

diff --git a/common/fastboot.c b/common/fastboot.c
index c800e89a65..7f5b8b6f03 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -337,6 +337,7 @@ int fastboot_handle_download_data(struct fastboot *fb, const void *buffer,
 void fastboot_download_finished(struct fastboot *fb)
 {
 	close(fb->download_fd);
+	fb->download_fd = 0;
 
 	printf("\n");
 
@@ -356,6 +357,11 @@ static void cb_download(struct fastboot *fb, const char *cmd)
 
 	init_progression_bar(fb->download_size);
 
+	if (fb->download_fd > 0) {
+		pr_err("%s called and %s is still opened\n", __func__, fb->tempname);
+		close(fb->download_fd);
+	}
+
 	fb->download_fd = open(fb->tempname, O_WRONLY | O_CREAT | O_TRUNC);
 	if (fb->download_fd < 0) {
 		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, "internal error");
-- 
2.27.0


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

[PATCH 15/21] fastboot: Add fastboot_abort()

[Sascha Hauer]

Add fastboot_abort() to allow aborting the current session.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/fastboot.c  | 12 ++++++++++++
 include/fastboot.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/common/fastboot.c b/common/fastboot.c
index 7f5b8b6f03..10ebb8b2a9 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -347,6 +347,18 @@ void fastboot_download_finished(struct fastboot *fb)
 	fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
 }
 
+void fastboot_abort(struct fastboot *fb)
+{
+	if (fb->download_fd > 0) {
+		close(fb->download_fd);
+		fb->download_fd = 0;
+	}
+
+	fb->active = false;
+
+	unlink(fb->tempname);
+}
+
 static void cb_download(struct fastboot *fb, const char *cmd)
 {
 	fb->download_size = simple_strtoul(cmd, NULL, 16);
diff --git a/include/fastboot.h b/include/fastboot.h
index 8619dcd34e..7718390ae5 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -66,4 +66,6 @@ int fastboot_tx_print(struct fastboot *fb, enum fastboot_msg_type type,
 void fastboot_start_download_generic(struct fastboot *fb);
 void fastboot_download_finished(struct fastboot *fb);
 void fastboot_exec_cmd(struct fastboot *fb, const char *cmdbuf);
+void fastboot_abort(struct fastboot *fb);
+
 #endif
-- 
2.27.0


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

[PATCH 16/21] fastboot: init list head in common

[Sascha Hauer]

The list of variables can be initialized in common code, no need to do
the in the different implementations.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/fastboot.c               | 2 ++
 drivers/usb/gadget/f_fastboot.c | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index 10ebb8b2a9..3f61208a21 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -171,6 +171,8 @@ int fastboot_generic_init(struct fastboot *fb, bool export_bbu)
 	struct file_list_entry *fentry;
 	struct fb_variable *var;
 
+	INIT_LIST_HEAD(&fb->variables);
+
 	var = fb_addvar(fb, "version");
 	fb_setvar(var, "0.4");
 	var = fb_addvar(fb, "bootloader-version");
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index f8a9c32530..35ad351f73 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -313,8 +313,6 @@ static struct usb_function *fastboot_alloc_func(struct usb_function_instance *fi
 
 	f_fb = xzalloc(sizeof(*f_fb));
 
-	INIT_LIST_HEAD(&f_fb->fastboot.variables);
-
 	f_fb->func.name = "fastboot";
 	f_fb->func.strings = fastboot_strings;
 	f_fb->func.bind = fastboot_bind;
-- 
2.27.0


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

[PATCH 17/21] fastboot net: implement fastboot over UDP

[Sascha Hauer]

From: Edmund Henniges <eh@emlix.com>

This implements the UDP variant of the fastboot protocol. The only way to
start the service for now is to compile with CONFIG_FASTBOOT_NET_ON_BOOT.
The service will bind to the network interface that provides the IPv4
gateway.

Sending an OKAY packet before performing a restart is necessary since
contrary to USB the host will not notice when a UDP server disappears.

Signed-off-by: Edmund Henniges <eh@emlix.com>
Signed-off-by: Daniel Glöckner <dg@emlix.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/fastboot.c      |   3 +
 include/fastboot.h     |   3 +
 include/fastboot_net.h |  12 +
 net/Kconfig            |  10 +
 net/Makefile           |   1 +
 net/fastboot.c         | 565 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 594 insertions(+)
 create mode 100644 include/fastboot_net.h
 create mode 100644 net/fastboot.c

diff --git a/common/fastboot.c b/common/fastboot.c
index 3f61208a21..d737107d19 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -245,6 +245,7 @@ static char *fastboot_msg[] = {
 	[FASTBOOT_MSG_FAIL] = "FAIL",
 	[FASTBOOT_MSG_INFO] = "INFO",
 	[FASTBOOT_MSG_DATA] = "DATA",
+	[FASTBOOT_MSG_NONE] = "",
 };
 
 int fastboot_tx_print(struct fastboot *fb, enum fastboot_msg_type type,
@@ -273,6 +274,7 @@ int fastboot_tx_print(struct fastboot *fb, enum fastboot_msg_type type,
 	case FASTBOOT_MSG_INFO:
 		pr_info("%pV\n", &vaf);
 		break;
+	case FASTBOOT_MSG_NONE:
 	case FASTBOOT_MSG_DATA:
 		break;
 	}
@@ -287,6 +289,7 @@ int fastboot_tx_print(struct fastboot *fb, enum fastboot_msg_type type,
 
 static void cb_reboot(struct fastboot *fb, const char *cmd)
 {
+	fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
 	restart_machine();
 }
 
diff --git a/include/fastboot.h b/include/fastboot.h
index 7718390ae5..fa826a1622 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -5,6 +5,8 @@
 #include <file-list.h>
 #include <net.h>
 
+#define FASTBOOT_MAX_CMD_LEN  64
+
 /*
  * Return codes for the exec_cmd callback above:
  *
@@ -51,6 +53,7 @@ enum fastboot_msg_type {
 	FASTBOOT_MSG_FAIL,
 	FASTBOOT_MSG_INFO,
 	FASTBOOT_MSG_DATA,
+	FASTBOOT_MSG_NONE,
 };
 
 extern int fastboot_bbu;
diff --git a/include/fastboot_net.h b/include/fastboot_net.h
new file mode 100644
index 0000000000..e4b9d98091
--- /dev/null
+++ b/include/fastboot_net.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef __FASTBOOT_NET__
+#define __FASTBOOT_NET__
+
+#include <fastboot.h>
+
+struct fastboot_net;
+
+struct fastboot_net *fastboot_net_init(struct fastboot_opts *opts);
+void fastboot_net_free(struct fastboot_net *fbn);
+
+#endif
diff --git a/net/Kconfig b/net/Kconfig
index 12b6bdb56d..a3c8c10f33 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -31,4 +31,14 @@ config NET_SNTP
 	bool
 	prompt "sntp support"
 
+config NET_FASTBOOT
+	bool
+	select BANNER
+	select FILE_LIST
+	select FASTBOOT_BASE
+	prompt "Android Fastboot support"
+	help
+	  This option adds support for the UDP variant of the Fastboot
+	  protocol.
+
 endif
diff --git a/net/Makefile b/net/Makefile
index eb8d439150..962b2dec58 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_CMD_PING)	+= ping.o
 obj-$(CONFIG_NET_RESOLV)+= dns.o
 obj-$(CONFIG_NET_NETCONSOLE) += netconsole.o
 obj-$(CONFIG_NET_IFUP)	+= ifup.o
+obj-$(CONFIG_NET_FASTBOOT) += fastboot.o
diff --git a/net/fastboot.c b/net/fastboot.c
new file mode 100644
index 0000000000..e839ee1838
--- /dev/null
+++ b/net/fastboot.c
@@ -0,0 +1,565 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Copyright 2020 Edmund Henniges <eh@emlix.com>
+ * Copyright 2020 Daniel Glöckner <dg@emlix.com>
+ * Ported from U-Boot to Barebox
+ */
+
+#define pr_fmt(fmt) "net fastboot: " fmt
+
+#include <common.h>
+#include <net.h>
+#include <fastboot.h>
+#include <fastboot_net.h>
+#include <environment.h>
+#include <progress.h>
+#include <unistd.h>
+#include <init.h>
+#include <work.h>
+#include <globalvar.h>
+#include <magicvar.h>
+
+#define FASTBOOT_PORT 5554
+#define MAX_MTU 1500
+#define PACKET_SIZE (min(PKTSIZE, MAX_MTU + ETHER_HDR_SIZE) \
+		      - (net_eth_to_udp_payload(0) - (char *)0))
+
+enum {
+	FASTBOOT_ERROR = 0,
+	FASTBOOT_QUERY = 1,
+	FASTBOOT_INIT = 2,
+	FASTBOOT_FASTBOOT = 3,
+};
+
+enum may_send {
+	MAY_NOT_SEND,
+	MAY_SEND_MESSAGE,
+	MAY_SEND_ACK,
+};
+
+struct __packed fastboot_header {
+	u8 id;
+	u8 flags;
+	u16 seq;
+};
+
+struct fastboot_net {
+	struct fastboot fastboot;
+
+	struct net_connection *net_con;
+	struct fastboot_header response_header;
+	struct poller_struct poller;
+	struct work_queue wq;
+	u64 host_waits_since;
+	u64 last_download_pkt;
+	bool sequence_number_seen;
+	bool active_download;
+	bool reinit;
+	bool send_keep_alive;
+	enum may_send may_send;
+
+	IPaddr_t host_addr;
+	u16 host_port;
+	u8 host_mac[ETH_ALEN];
+	u16 sequence_number;
+	u16 last_payload_len;
+	uchar last_payload[FASTBOOT_MAX_CMD_LEN + sizeof(struct fastboot_header)];
+};
+
+static const ushort udp_version = 1;
+
+static bool is_current_connection(struct fastboot_net *fbn)
+{
+	return fbn->host_addr == net_read_ip(&fbn->net_con->ip->daddr) &&
+	       fbn->host_port == fbn->net_con->udp->uh_dport;
+}
+
+static void fastboot_net_abort(struct fastboot_net *fbn)
+{
+	fbn->reinit = true;
+
+	fastboot_abort(&fbn->fastboot);
+
+	fbn->active_download = false;
+
+	poller_unregister(&fbn->poller);
+
+	/*
+	 * If the host sends a data packet at a time when an empty packet was
+	 * expected, fastboot_abort is called and an error message is sent.
+	 * We don't want to execute the contents of the bad packet afterwards.
+	 * Clearing command also tells our keep-alive poller to stop sending
+	 * messages.
+	 */
+	wq_cancel_work(&fbn->wq);
+}
+
+static void fastboot_send(struct fastboot_net *fbn,
+			  struct fastboot_header header,
+			  const char *error_msg)
+{
+	short tmp;
+	uchar *packet = net_udp_get_payload(fbn->net_con);
+	uchar *packet_base = packet;
+	bool current_session = false;
+
+	if (fbn->sequence_number == ntohs(header.seq) &&
+	    is_current_connection(fbn))
+		current_session = true;
+
+	if (error_msg)
+		header.id = FASTBOOT_ERROR;
+
+	/* send header */
+	memcpy(packet, &header, sizeof(header));
+	packet += sizeof(header);
+
+	switch (header.id) {
+	case FASTBOOT_QUERY:
+		/* send sequence number */
+		tmp = htons(fbn->sequence_number);
+		memcpy(packet, &tmp, sizeof(tmp));
+		packet += sizeof(tmp);
+		break;
+	case FASTBOOT_INIT:
+		/* send udp version and packet size */
+		tmp = htons(udp_version);
+		memcpy(packet, &tmp, sizeof(tmp));
+		packet += sizeof(tmp);
+		tmp = htons(PACKET_SIZE);
+		memcpy(packet, &tmp, sizeof(tmp));
+		packet += sizeof(tmp);
+		break;
+	case FASTBOOT_ERROR:
+		pr_err("%s\n", error_msg);
+
+		/* send error message */
+		tmp = strlen(error_msg);
+		memcpy(packet, error_msg, tmp);
+		packet += tmp;
+
+		if (current_session)
+			fastboot_net_abort(fbn);
+
+		break;
+	}
+
+	if (current_session && header.id != FASTBOOT_QUERY) {
+		fbn->sequence_number++;
+		fbn->sequence_number_seen = false;
+		fbn->last_payload_len = packet - packet_base;
+		memcpy(fbn->last_payload, packet_base, fbn->last_payload_len);
+	}
+	net_udp_send(fbn->net_con, packet - packet_base);
+}
+
+static int fastboot_net_wait_may_send(struct fastboot_net *fbn)
+{
+	uint64_t start = get_time_ns();
+
+	while (!is_timeout(start, 2 * SECOND)) {
+		if (fbn->may_send != MAY_NOT_SEND)
+			return 0;
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int fastboot_write_net(struct fastboot *fb, const char *buf,
+			      unsigned int n)
+{
+	struct fastboot_net *fbn = container_of(fb, struct fastboot_net,
+						fastboot);
+	struct fastboot_header response_header;
+	uchar *packet;
+	uchar *packet_base;
+	int ret;
+
+	if (fbn->reinit)
+		return 0;
+
+	/*
+	 * This function is either called in command context, in which
+	 * case we may wait, or from the keepalive poller which explicitly
+	 * only calls us when we don't have to wait here.
+	 */
+	ret = fastboot_net_wait_may_send(fbn);
+	if (ret) {
+		fastboot_net_abort(fbn);
+		return ret;
+	}
+
+	if (n && fbn->may_send == MAY_SEND_ACK) {
+		fastboot_send(fbn, fbn->response_header,
+				"Have message but only ACK allowed");
+		return -EPROTO;
+	} else if (!n && fbn->may_send == MAY_SEND_MESSAGE) {
+		fastboot_send(fbn, fbn->response_header,
+				"Want to send ACK but message expected");
+		return -EPROTO;
+	}
+
+	response_header = fbn->response_header;
+	response_header.flags = 0;
+	response_header.seq = htons(fbn->sequence_number);
+	++fbn->sequence_number;
+	fbn->sequence_number_seen = false;
+
+	packet = net_udp_get_payload(fbn->net_con);
+	packet_base = packet;
+
+	/* Write headers */
+	memcpy(packet, &response_header, sizeof(response_header));
+	packet += sizeof(response_header);
+	/* Write response */
+	memcpy(packet, buf, n);
+	packet += n;
+
+	/* Save packet for retransmitting */
+	fbn->last_payload_len = packet - packet_base;
+	memcpy(fbn->last_payload, packet_base, fbn->last_payload_len);
+
+	memcpy(fbn->net_con->et->et_dest, fbn->host_mac, ETH_ALEN);
+	net_write_ip(&fbn->net_con->ip->daddr, fbn->host_addr);
+	fbn->net_con->udp->uh_dport = fbn->host_port;
+	net_udp_send(fbn->net_con, fbn->last_payload_len);
+
+	fbn->may_send = MAY_NOT_SEND;
+
+	return 0;
+}
+
+static void fastboot_start_download_net(struct fastboot *fb)
+{
+	struct fastboot_net *fbn = container_of(fb, struct fastboot_net,
+						fastboot);
+
+	fastboot_start_download_generic(fb);
+	fbn->active_download = true;
+	fbn->last_download_pkt = get_time_ns();
+}
+
+/* must send exactly one packet on all code paths */
+static void fastboot_data_download(struct fastboot_net *fbn,
+				   const void *fastboot_data,
+				   unsigned int fastboot_data_len)
+{
+	int ret;
+
+	if (fastboot_data_len == 0 ||
+	    (fbn->fastboot.download_bytes + fastboot_data_len) >
+	    fbn->fastboot.download_size) {
+		fastboot_send(fbn, fbn->response_header,
+			      "Received invalid data length");
+		return;
+	}
+
+	ret = fastboot_handle_download_data(&fbn->fastboot, fastboot_data,
+					    fastboot_data_len);
+	if (ret < 0) {
+		fastboot_send(fbn, fbn->response_header, strerror(-ret));
+		return;
+	}
+
+	fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_NONE, "");
+}
+
+struct fastboot_work {
+	struct work_struct work;
+	struct fastboot_net *fbn;
+	bool download_finished;
+	char command[FASTBOOT_MAX_CMD_LEN + 1];
+};
+
+static void fastboot_handle_type_fastboot(struct fastboot_net *fbn,
+					  struct fastboot_header header,
+					  char *fastboot_data,
+					  unsigned int fastboot_data_len)
+{
+	struct fastboot_work *w;
+
+	fbn->response_header = header;
+	fbn->host_waits_since = get_time_ns();
+	fbn->may_send = fastboot_data_len ? MAY_SEND_ACK : MAY_SEND_MESSAGE;
+
+	if (fbn->active_download) {
+		fbn->last_download_pkt = get_time_ns();
+
+		if (!fastboot_data_len && fbn->fastboot.download_bytes
+					   == fbn->fastboot.download_size) {
+
+			fbn->active_download = false;
+
+			w = xzalloc(sizeof(*w));
+			w->fbn = fbn;
+			w->download_finished = true;
+
+			wq_queue_work(&fbn->wq, &w->work);
+		} else {
+			fastboot_data_download(fbn, fastboot_data,
+					       fastboot_data_len);
+		}
+		return;
+	}
+
+	if (fastboot_data_len >= FASTBOOT_MAX_CMD_LEN) {
+		fastboot_send(fbn, header, "command too long");
+		return;
+	}
+
+	if (!list_empty(&fbn->wq.work))
+		return;
+
+	if (fastboot_data_len) {
+		w = xzalloc(sizeof(*w));
+		w->fbn = fbn;
+		memcpy(w->command, fastboot_data, fastboot_data_len);
+		w->command[fastboot_data_len] = 0;
+
+		wq_queue_work(&fbn->wq, &w->work);
+	}
+}
+
+static void fastboot_check_retransmit(struct fastboot_net *fbn,
+				      struct fastboot_header header)
+{
+	if (ntohs(header.seq) == fbn->sequence_number - 1 &&
+	    is_current_connection(fbn)) {
+		/* Retransmit last sent packet */
+		memcpy(net_udp_get_payload(fbn->net_con),
+		       fbn->last_payload, fbn->last_payload_len);
+		net_udp_send(fbn->net_con, fbn->last_payload_len);
+	}
+}
+
+static void fastboot_handler(void *ctx, char *packet, unsigned int raw_len)
+{
+	unsigned int len = net_eth_to_udplen(packet);
+	struct ethernet *eth_header = (struct ethernet *)packet;
+	struct iphdr *ip_header = net_eth_to_iphdr(packet);
+	struct udphdr *udp_header = net_eth_to_udphdr(packet);
+	char *payload = net_eth_to_udp_payload(packet);
+	struct fastboot_net *fbn = ctx;
+	struct fastboot_header header;
+	char *fastboot_data = payload + sizeof(header);
+	u16 tot_len = ntohs(ip_header->tot_len);
+	int ret;
+
+	/* catch bogus tot_len values */
+	if ((char *)ip_header - packet + tot_len > raw_len)
+		return;
+
+	/* catch packets split into fragments that are too small to reply */
+	if (fastboot_data - (char *)ip_header > tot_len)
+		return;
+
+	/* catch packets too small to be valid */
+	if (len < sizeof(struct fastboot_header))
+		return;
+
+	memcpy(&header, payload, sizeof(header));
+	header.flags = 0;
+	len -= sizeof(header);
+
+	/* catch remaining fragmented packets */
+	if (fastboot_data - (char *)ip_header + len > tot_len) {
+		fastboot_send(fbn, header,
+			      "can't reassemble fragmented frames");
+		return;
+	}
+	/* catch too large packets */
+	if (len > PACKET_SIZE) {
+		fastboot_send(fbn, header, "packet too large");
+		return;
+	}
+
+	memcpy(fbn->net_con->et->et_dest, eth_header->et_src, ETH_ALEN);
+	net_copy_ip(&fbn->net_con->ip->daddr, &ip_header->saddr);
+	fbn->net_con->udp->uh_dport = udp_header->uh_sport;
+
+	switch (header.id) {
+	case FASTBOOT_QUERY:
+		fastboot_send(fbn, header, NULL);
+		break;
+	case FASTBOOT_INIT:
+		if (ntohs(header.seq) != fbn->sequence_number) {
+			fastboot_check_retransmit(fbn, header);
+			break;
+		}
+		fbn->host_addr = net_read_ip(&ip_header->saddr);
+		fbn->host_port = udp_header->uh_sport;
+		memcpy(fbn->host_mac, eth_header->et_src, ETH_ALEN);
+		fastboot_net_abort(fbn);
+		ret = poller_register(&fbn->poller, "fastboot");
+		if (ret) {
+			pr_err("Cannot register poller: %s\n", strerror(-ret));
+			return;
+		}
+		fastboot_send(fbn, header, NULL);
+		break;
+	case FASTBOOT_FASTBOOT:
+		if (!is_current_connection(fbn))
+			break;
+		memcpy(fbn->host_mac, eth_header->et_src, ETH_ALEN);
+
+		if (ntohs(header.seq) != fbn->sequence_number) {
+			fastboot_check_retransmit(fbn, header);
+		} else if (!fbn->sequence_number_seen) {
+			fbn->sequence_number_seen = true;
+			fastboot_handle_type_fastboot(fbn, header,
+						      fastboot_data, len);
+		}
+		break;
+	default:
+		fastboot_send(fbn, header, "unknown packet type");
+		break;
+	}
+}
+
+static void fastboot_do_work(struct work_struct *w)
+{
+	struct fastboot_work *fw = container_of(w, struct fastboot_work, work);
+	struct fastboot_net *fbn = fw->fbn;
+
+	if (fw->download_finished) {
+		fastboot_download_finished(&fbn->fastboot);
+		goto out;
+	}
+
+	fbn->reinit = false;
+	fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_NONE, "");
+
+	fbn->send_keep_alive = true;
+
+	fastboot_exec_cmd(&fbn->fastboot, fw->command);
+	fbn->send_keep_alive = false;
+out:
+	free(fw);
+}
+
+static void fastboot_work_cancel(struct work_struct *w)
+{
+	struct fastboot_work *fw = container_of(w, struct fastboot_work, work);
+
+	free(fw);
+}
+
+static void fastboot_poll(struct poller_struct *poller)
+{
+	struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
+					       poller);
+
+	if (fbn->active_download && is_timeout(fbn->last_download_pkt, 5 * SECOND)) {
+		pr_err("No progress for 5s, aborting\n");
+		fastboot_net_abort(fbn);
+		return;
+	}
+
+	if (!fbn->send_keep_alive)
+		return;
+
+	if (!is_timeout_non_interruptible(fbn->host_waits_since,
+					 30ULL * NSEC_PER_SEC))
+		return;
+
+	if (fbn->may_send != MAY_SEND_MESSAGE)
+		return;
+
+	fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_INFO, "still busy");
+}
+
+void fastboot_net_free(struct fastboot_net *fbn)
+{
+	fastboot_generic_close(&fbn->fastboot);
+	net_unregister(fbn->net_con);
+	fastboot_generic_free(&fbn->fastboot);
+	wq_unregister(&fbn->wq);
+	free(fbn);
+}
+
+struct fastboot_net *fastboot_net_init(struct fastboot_opts *opts)
+{
+	struct fastboot_net *fbn;
+	int ret;
+
+	fbn = xzalloc(sizeof(*fbn));
+	fbn->fastboot.write = fastboot_write_net;
+	fbn->fastboot.start_download = fastboot_start_download_net;
+
+	if (opts) {
+		fbn->fastboot.files = opts->files;
+		fbn->fastboot.cmd_exec = opts->cmd_exec;
+		fbn->fastboot.cmd_flash = opts->cmd_flash;
+		ret = fastboot_generic_init(&fbn->fastboot, opts->export_bbu);
+	} else {
+		fbn->fastboot.files = file_list_parse(fastboot_partitions
+						      ? fastboot_partitions
+						      : "");
+		ret = fastboot_generic_init(&fbn->fastboot, fastboot_bbu);
+	}
+	if (ret)
+		goto fail_generic_init;
+
+	fbn->net_con = net_udp_new(IP_BROADCAST, FASTBOOT_PORT,
+				   fastboot_handler, fbn);
+	if (IS_ERR(fbn->net_con)) {
+		ret = PTR_ERR(fbn->net_con);
+		goto fail_net_con;
+	}
+	net_udp_bind(fbn->net_con, FASTBOOT_PORT);
+
+	eth_open(fbn->net_con->edev);
+
+	fbn->poller.func = fastboot_poll;
+
+	fbn->wq.fn = fastboot_do_work;
+	fbn->wq.cancel = fastboot_work_cancel;
+
+	wq_register(&fbn->wq);
+
+	return fbn;
+
+fail_net_con:
+	fastboot_generic_free(&fbn->fastboot);
+fail_generic_init:
+	free(fbn);
+	return ERR_PTR(ret);
+}
+
+static struct fastboot_net *fastboot_net_obj;
+static int fastboot_net_autostart;
+
+static int fastboot_on_boot(void)
+{
+	struct fastboot_net *fbn;
+
+	globalvar_add_simple_bool("fastboot.net.autostart",
+				  &fastboot_net_autostart);
+
+	if (!fastboot_net_autostart)
+		return 0;
+
+	ifup_all(0);
+	fbn = fastboot_net_init(NULL);
+
+	if (IS_ERR(fbn))
+		return PTR_ERR(fbn);
+
+	fastboot_net_obj = fbn;
+	return 0;
+}
+
+static void fastboot_net_exit(void)
+{
+	if (fastboot_net_obj)
+		fastboot_net_free(fastboot_net_obj);
+}
+
+postenvironment_initcall(fastboot_on_boot);
+predevshutdown_exitcall(fastboot_net_exit);
+
+BAREBOX_MAGICVAR_NAMED(global_fastboot_net_autostart,
+		       global.fastboot.net.autostart,
+		       "If true, automatically start fastboot over UDP during startup");
-- 
2.27.0


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

[PATCH 18/21] usb: fastboot: execute commands in command context

[Sascha Hauer]

Long lived commands which also use other resources shouldn't be executed
in a poller. Use a workqeue to delay the work to command context.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/f_fastboot.c | 52 ++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 35ad351f73..da3e0c7a8f 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -21,6 +21,7 @@
 #define pr_fmt(fmt) "fastboot: " fmt
 
 #include <dma.h>
+#include <work.h>
 #include <unistd.h>
 #include <progress.h>
 #include <fastboot.h>
@@ -39,6 +40,7 @@ struct f_fastboot {
 	/* IN/OUT EP's and corresponding requests */
 	struct usb_ep *in_ep, *out_ep;
 	struct usb_request *in_req, *out_req;
+	struct work_queue wq;
 };
 
 static inline struct f_fastboot *func_to_fastboot(struct usb_function *f)
@@ -136,6 +138,32 @@ static void fastboot_complete(struct usb_ep *ep, struct usb_request *req)
 {
 }
 
+struct fastboot_work {
+	struct work_struct work;
+	struct f_fastboot *f_fb;
+	char command[FASTBOOT_MAX_CMD_LEN + 1];
+};
+
+static void fastboot_do_work(struct work_struct *w)
+{
+	struct fastboot_work *fw = container_of(w, struct fastboot_work, work);
+	struct f_fastboot *f_fb = fw->f_fb;
+
+	fastboot_exec_cmd(&f_fb->fastboot, fw->command);
+
+	memset(f_fb->out_req->buf, 0, EP_BUFFER_SIZE);
+	usb_ep_queue(f_fb->out_ep, f_fb->out_req);
+
+	free(fw);
+}
+
+static void fastboot_work_cancel(struct work_struct *w)
+{
+	struct fastboot_work *fw = container_of(w, struct fastboot_work, work);
+
+	free(fw);
+}
+
 static struct usb_request *fastboot_alloc_request(struct usb_ep *ep)
 {
 	struct usb_request *req;
@@ -172,6 +200,11 @@ static int fastboot_bind(struct usb_configuration *c, struct usb_function *f)
 	f_fb->fastboot.cmd_exec = opts->common.cmd_exec;
 	f_fb->fastboot.cmd_flash = opts->common.cmd_flash;
 
+	f_fb->wq.fn = fastboot_do_work;
+	f_fb->wq.cancel = fastboot_work_cancel;
+
+	wq_register(&f_fb->wq);
+
 	ret = fastboot_generic_init(&f_fb->fastboot, opts->common.export_bbu);
 	if (ret)
 		return ret;
@@ -246,6 +279,8 @@ static void fastboot_unbind(struct usb_configuration *c, struct usb_function *f)
 	usb_ep_free_request(f_fb->out_ep, f_fb->out_req);
 	f_fb->out_req = NULL;
 
+	wq_unregister(&f_fb->wq);
+
 	fastboot_generic_free(&f_fb->fastboot);
 }
 
@@ -428,16 +463,19 @@ static void fastboot_start_download_usb(struct fastboot *fb)
 
 static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
 {
-	char *cmdbuf = req->buf;
 	struct f_fastboot *f_fb = req->context;
+	struct fastboot_work *w;
+	int len;
 
 	if (req->status != 0)
 		return;
 
-	*(cmdbuf + req->actual) = 0;
-	fastboot_exec_cmd(&f_fb->fastboot, cmdbuf);
-	*cmdbuf = '\0';
-	req->actual = 0;
-	memset(req->buf, 0, EP_BUFFER_SIZE);
-	usb_ep_queue(ep, req);
+	w = xzalloc(sizeof(*w));
+	w->f_fb = f_fb;
+
+	len = min_t(unsigned int, req->actual, FASTBOOT_MAX_CMD_LEN);
+
+	memcpy(w->command, req->buf, len);
+
+	wq_queue_work(&f_fb->wq, &w->work);
 }
-- 
2.27.0


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

[PATCH 19/21] Add WARN_ONCE() macro

[Sascha Hauer]

This adds WARN_ONCE from the Linux Kernel. It is useful to warn only
once when we would otherwise spam the log.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/asm-generic/bug.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 8583d2425f..82b78261fc 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -34,4 +34,17 @@
 	unlikely(__ret_warn_on);					\
 })
 #endif
+
+#define WARN_ONCE(condition, format...) ({	\
+	static int __warned;			\
+	int __ret_warn_once = !!(condition);	\
+						\
+	if (unlikely(__ret_warn_once)) {	\
+		if (WARN(!__warned, format)) {	\
+			__warned = 1;		\
+			dump_stack();		\
+		}				\
+	}					\
+	unlikely(__ret_warn_once);		\
+})
 #endif
-- 
2.27.0


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

[PATCH 20/21] fs: Warn when filesystem operations are called from a poller

[Sascha Hauer]

Filesystem operations possibly call into arbitrary devices, so shouldn't
be used from a poller. This patch sprinkles some WARN_ONCE() when this
happens. One exception is when the file which is accessed is on ramfs
which doesn't have any dependencies to devices.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/poller.c |  2 +-
 fs/fs.c         | 35 +++++++++++++++++++++++++++++++++++
 include/slice.h |  6 ++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/common/poller.c b/common/poller.c
index 7b1b92714c..50c518697b 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -16,7 +16,7 @@
 #include <slice.h>
 
 static LIST_HEAD(poller_list);
-static int poller_active;
+int poller_active;
 
 int poller_register(struct poller_struct *poller, const char *name)
 {
diff --git a/fs/fs.c b/fs/fs.c
index e04cadfe5d..d9bef0f852 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -31,6 +31,7 @@
 #include <environment.h>
 #include <libgen.h>
 #include <block.h>
+#include <slice.h>
 #include <libfile.h>
 #include <parseopt.h>
 #include <linux/namei.h>
@@ -74,6 +75,8 @@ static FILE *files;
 static struct dentry *d_root;
 static struct vfsmount *mnt_root;
 
+static struct fs_driver_d *ramfs_driver;
+
 static int init_fs(void)
 {
 	cwd = xzalloc(PATH_MAX);
@@ -250,6 +253,7 @@ static ssize_t __read(FILE *f, void *buf, size_t count)
 	struct fs_driver_d *fsdrv;
 	int ret;
 
+
 	if ((f->flags & O_ACCMODE) == O_WRONLY) {
 		ret = -EBADF;
 		goto out;
@@ -257,6 +261,9 @@ static ssize_t __read(FILE *f, void *buf, size_t count)
 
 	fsdrv = f->fsdev->driver;
 
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (f->size != FILE_SIZE_STREAM && f->pos + count > f->size)
 		count = f->size - f->pos;
 
@@ -315,6 +322,10 @@ static ssize_t __write(FILE *f, const void *buf, size_t count)
 	}
 
 	fsdrv = f->fsdev->driver;
+
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (f->size != FILE_SIZE_STREAM && f->pos + count > f->size) {
 		ret = fsdrv->truncate(&f->fsdev->dev, f, f->pos + count);
 		if (ret) {
@@ -402,6 +413,9 @@ loff_t lseek(int fd, loff_t offset, int whence)
 
 	fsdrv = f->fsdev->driver;
 
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	ret = -EINVAL;
 
 	switch (whence) {
@@ -457,6 +471,10 @@ int erase(int fd, loff_t count, loff_t offset)
 		return -EINVAL;
 
 	fsdrv = f->fsdev->driver;
+
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (fsdrv->erase)
 		ret = fsdrv->erase(&f->fsdev->dev, f, count, offset);
 	else
@@ -483,6 +501,10 @@ int protect(int fd, size_t count, loff_t offset, int prot)
 		count = f->size - offset;
 
 	fsdrv = f->fsdev->driver;
+
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (fsdrv->protect)
 		ret = fsdrv->protect(&f->fsdev->dev, f, count, offset, prot);
 	else
@@ -509,6 +531,10 @@ int discard_range(int fd, loff_t count, loff_t offset)
 		count = f->size - offset;
 
 	fsdrv = f->fsdev->driver;
+
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (fsdrv->discard_range)
 		ret = fsdrv->discard_range(&f->fsdev->dev, f, count, offset);
 	else
@@ -547,6 +573,9 @@ void *memmap(int fd, int flags)
 
 	fsdrv = f->fsdev->driver;
 
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (fsdrv->memmap)
 		ret = fsdrv->memmap(&f->fsdev->dev, f, &retp, flags);
 	else
@@ -570,6 +599,9 @@ int close(int fd)
 
 	fsdrv = f->fsdev->driver;
 
+	if (fsdrv != ramfs_driver)
+		assert_command_context();
+
 	if (fsdrv->close)
 		ret = fsdrv->close(&f->fsdev->dev, f);
 
@@ -700,6 +732,9 @@ int register_fs_driver(struct fs_driver_d *fsdrv)
 	fsdrv->drv.bus = &fs_bus;
 	register_driver(&fsdrv->drv);
 
+	if (!strcmp(fsdrv->drv.name, "ramfs"))
+		ramfs_driver = fsdrv;
+
 	return 0;
 }
 EXPORT_SYMBOL(register_fs_driver);
diff --git a/include/slice.h b/include/slice.h
index fd753e194b..5438aeef90 100644
--- a/include/slice.h
+++ b/include/slice.h
@@ -33,4 +33,10 @@ extern struct slice command_slice;
 void command_slice_acquire(void);
 void command_slice_release(void);
 
+extern int poller_active;
+
+#define assert_command_context() ({    \
+	WARN_ONCE(poller_active, "%s called in poller\n", __func__); \
+})
+
 #endif /* __SLICE_H */
-- 
2.27.0


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

[PATCH 21/21] Documentation: Add document for parallel execution in barebox

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/devel/background-execution.rst | 123 +++++++++++++++++++
 Documentation/devel/devel.rst                |  14 +++
 Documentation/index.rst                      |   1 +
 3 files changed, 138 insertions(+)
 create mode 100644 Documentation/devel/background-execution.rst
 create mode 100644 Documentation/devel/devel.rst

diff --git a/Documentation/devel/background-execution.rst b/Documentation/devel/background-execution.rst
new file mode 100644
index 0000000000..29ec2236e7
--- /dev/null
+++ b/Documentation/devel/background-execution.rst
@@ -0,0 +1,123 @@
+Background execution in barebox
+===============================
+
+barebox is single-threaded and doesn't support interrupts. Nevertheless it is
+sometimes desired to execute code "in the background", like for example polling
+for completion of transfers or to regularly blink a heartbeat LED.  For these
+scenarios barebox offers the techniques described below.
+
+Pollers
+-------
+
+Pollers are a way in barebox to frequently execute code in the background.
+barebox is single-threaded, so a poller is not executed as a separate thread,
+but instead pollers are executed whenever ``is_timeout()`` is called.  This has
+a few implications. First of all, pollers are not executed when
+``is_timeout()`` is not called. For this and other reasons, loops polling for
+hardware events should always use a timeout, which is best implemented with
+``is_timeout()``. Another thing to remember is that pollers can be executed
+anywhere in the middle of other device accesses whenever ``is_timeout()`` is
+called. Care must be taken that a poller doesn't access the very same device
+again itself. See "slices" below on how devices can safely be accessed from
+pollers. Some operations are entirely forbidden from pollers. For example it is
+forbidden to access the virtual filesystem, as this could trigger arbitrary
+device accesses.  The macro ``assert_command_context()`` is added to those
+places to make sure they are not called in the wrong context. The poller
+interface is declared in ``include/poller.h``.  ``poller_register()`` is used
+to register a poller. The ``struct poller_struct`` passed to
+``poller_register()`` needs to have the ``poller_struct.func()`` member
+populated with the function that shall be called. From the moment a poller is
+registered ``poller_struct.func()`` will be called regularly as long as someone
+calls ``is_timeout()``.  A special poller can be registered with
+``poller_async_register()``. A poller registered this way won't be called right
+away, instead running it can be triggered by calling ``poller_call_async()``.
+This will execute the poller after the ``@delay_ns`` argument.
+``poller_call_async()`` may also be called from with the poller, so with this
+it's possible to run a poller regularly with configurable delays.
+
+Pollers are limited in the things they can do. Poller code must always be
+prepared for the case that the resources it accesses are currently busy and
+handle this gracefully by trying again later. Most places in barebox either do
+not test for resources being busy or return with an error if they are busy.
+Calling into the filesystem potentially uses arbitrary other devices, so
+calling into the filesystem from pollers is forbidden. For the same reason it
+is forbidden to execute barebox commands from pollers.
+
+Workqueues
+----------
+
+Sometimes code wants to access files from poller context or even wants to
+execute barebox commands from poller context. The fastboot implementation is an
+example for such a case. The fastboot commands come in via USB or network and
+the completion and packet receive handlers are executed in poller context. Here
+workqueues come into play. They allow to queue work items from poller context.
+These work items are executed later at the point where the shell fetches its
+next command. At this point we implicitly know that it's allowed to execute
+commands or to access the filesystem, because otherwise the shell could not do
+it. This means that execution of the next shell command will be delayed until
+all work items are being done. Likewise the work items will only be executed
+when the current shell command has finished.
+
+The workqueue interface is declared in ``include/work.h``.
+
+``wq_register()`` is the first step to use the workqueue interface.
+``wq_register()`` registers a workqueue to which work items can be attached.
+Before registering a workqueue a ``struct work_queue`` has to be populated with
+two function callbacks.  ``work_queue.fn()`` is the callback that actually does
+the work once it is scheduled.  ``work_queue.cancel()`` is a callback which is
+executed when the pending work shall be cancelled. This is primarily for
+freeing the memory associated to a work item.  ``wq_queue_work()`` is for
+actually queueing a work item on a work queue. This can be called from poller
+code. Usually a work item is allocated by the poller and then freed either in
+``work_queue.fn()`` or in ``work_queue.cancel()``.
+
+Slices
+------
+
+Slices are a way to check if a device is currently busy and thus may not be
+called into currently. Pollers wanting to access a device must call
+``slice_busy()`` on the slice provided by the device before calling into it.
+When the slice is acquired (which can only happen inside a poller) the poller
+can't continue at this moment and must try again next time it is executed.
+Drivers whose devices provide a slice must call ``slice_busy()`` before
+accessing the device and ``slice_release()`` afterwards. Slices can have
+dependencies to other slices, for example a USB network controller uses the
+corresponding USB host controller. A dependency can be expressed with
+``slice_depends_on()``. With this the USB network controller can add a
+dependency from the network device it provides itself to the USB host
+controller it depends on.  With this ``slice_busy()`` on the network device
+will return ``true`` when the USB host controller is busy.
+
+The usual pattern for using slices is that the device driver for a device
+calls ``slice_acquire()`` when entering the driver and ``slice_release()``
+before leaving the driver. The driver also provides a function returning
+the slice for a device, for example the ethernet support code provides
+``struct slice *eth_device_slice(struct eth_device *edev)``. Poller code
+which wants to use the ethernet device checks for the availability doing
+``slice_busy(eth_device_slice(edev))`` before accessing the ethernet
+device.
+
+Limitations
+-----------
+
+What barebox does is best described as cooperative multitasking. As pollers
+can't be interrupted, it will directly affect the user experience when they
+take too long. When barebox reacts sluggishly to key presses, then probably
+pollers likely take too long to execute. A first test if this is the case can
+be done by executing ``poller -t`` on the command line. This command will print
+how many times we can execute all registered pollers in one second. When this
+number is too low then pollers are guilty responsible. Workqueues help to run
+schedule/execute longer running code, but during the time while workqueues are
+executed nothing else happens. This means that when fastboot flashes an image
+in a workqueue then barebox won't react to any key presses on the command line.
+The usage of the interfaces described in this document is not yet very
+widespread in barebox. The interfaces are used in the places where we need
+them, but there are other places which do not use them but should. For example
+using a LED driven by a I2C GPIO exander used as hearbeat LED won't work
+properly currently. Consider the I2C driver accesses an unrelated I2C device,
+like an EEPROM. After having initiated the transfer the driver polls for the
+transfer being completed using a ``is_timeout()`` loop. The call to
+``is_timeout()`` then calls into the registered pollers from which one accesses
+the same I2C bus whose driver is just polling for completion of another
+transfer. With this the I2C driver is in an undefined state and will likely not
+work anymore.
diff --git a/Documentation/devel/devel.rst b/Documentation/devel/devel.rst
new file mode 100644
index 0000000000..f559512ca3
--- /dev/null
+++ b/Documentation/devel/devel.rst
@@ -0,0 +1,14 @@
+.. highlight:: console
+
+barebox programming
+===================
+
+Contents:
+
+.. toctree::
+   :maxdepth: 2
+
+   background-execution
+
+* :ref:`search`
+* :ref:`genindex`
diff --git a/Documentation/index.rst b/Documentation/index.rst
index b7dae2396b..836dc41af2 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -19,6 +19,7 @@ Contents:
    boards
    glossary
    devicetree/*
+   devel/devel.rst
 
 * :ref:`search`
 * :ref:`genindex`
-- 
2.27.0


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

Re: [PATCH 17/21] fastboot net: implement fastboot over UDP

[Daniel Glöckner]

Hello Sascha,

Am 19.06.20 um 09:44 schrieb Sascha Hauer:
> +struct fastboot_net {
> +	struct fastboot fastboot;
> +
> +	struct net_connection *net_con;
> +	struct fastboot_header response_header;
> +	struct poller_struct poller;
> +	struct work_queue wq;
> +	u64 host_waits_since;
> +	u64 last_download_pkt;
> +	bool sequence_number_seen;
> +	bool active_download;
> +	bool reinit;
> +	bool send_keep_alive;
> +	enum may_send may_send;
> +
> +	IPaddr_t host_addr;
> +	u16 host_port;
> +	u8 host_mac[ETH_ALEN];
> +	u16 sequence_number;
> +	u16 last_payload_len;
> +	uchar last_payload[FASTBOOT_MAX_CMD_LEN + sizeof(struct fastboot_header)];

This is not FASTBOOT_MAX_CMD_LEN. It's the 64 that is strewn around in
fastboot_tx_print. Adding a new constant FASTBOOT_MAX_MSG_LEN would be
correct.

[...]

> +static int fastboot_write_net(struct fastboot *fb, const char *buf,
> +			      unsigned int n)
> +{
> +	struct fastboot_net *fbn = container_of(fb, struct fastboot_net,
> +						fastboot);
> +	struct fastboot_header response_header;
> +	uchar *packet;
> +	uchar *packet_base;
> +	int ret;
> +
> +	if (fbn->reinit)
> +		return 0;
> +
> +	/*
> +	 * This function is either called in command context, in which
> +	 * case we may wait, or from the keepalive poller which explicitly
> +	 * only calls us when we don't have to wait here.
> +	 */
> +	ret = fastboot_net_wait_may_send(fbn);
> +	if (ret) {
> +		fastboot_net_abort(fbn);
> +		return ret;
> +	}
> +
> +	if (n && fbn->may_send == MAY_SEND_ACK) {
> +		fastboot_send(fbn, fbn->response_header,
> +				"Have message but only ACK allowed");
> +		return -EPROTO;
> +	} else if (!n && fbn->may_send == MAY_SEND_MESSAGE) {
> +		fastboot_send(fbn, fbn->response_header,
> +				"Want to send ACK but message expected");
> +		return -EPROTO;
> +	}
> +
> +	response_header = fbn->response_header;
> +	response_header.flags = 0;
> +	response_header.seq = htons(fbn->sequence_number);
> +	++fbn->sequence_number;
> +	fbn->sequence_number_seen = false;
> +
> +	packet = net_udp_get_payload(fbn->net_con);
> +	packet_base = packet;
> +
> +	/* Write headers */
> +	memcpy(packet, &response_header, sizeof(response_header));
> +	packet += sizeof(response_header);
> +	/* Write response */
> +	memcpy(packet, buf, n);
> +	packet += n;
> +
> +	/* Save packet for retransmitting */
> +	fbn->last_payload_len = packet - packet_base;
> +	memcpy(fbn->last_payload, packet_base, fbn->last_payload_len);
> +
> +	memcpy(fbn->net_con->et->et_dest, fbn->host_mac, ETH_ALEN);
> +	net_write_ip(&fbn->net_con->ip->daddr, fbn->host_addr);
> +	fbn->net_con->udp->uh_dport = fbn->host_port;
> +	net_udp_send(fbn->net_con, fbn->last_payload_len);
> +
> +	fbn->may_send = MAY_NOT_SEND;

You moved that line below net_udp_send. Is there any risk that

1. our work queue executes a command which calls fastboot_tx_print
2. the net_udp_send caused by that fastboot_tx_print sleeps
3. our poller is executed and decides to send a message because
   may_send is still MAY_SEND_MESSAGE

?

[...]

> +static void fastboot_start_download_net(struct fastboot *fb)
> +{
> +	struct fastboot_net *fbn = container_of(fb, struct fastboot_net,
> +						fastboot);
> +
> +	fastboot_start_download_generic(fb);
> +	fbn->active_download = true;
> +	fbn->last_download_pkt = get_time_ns();
> +}

Although you added that last_download_pkt timeout check to the poller,
there is still the risk that we will never close download_fd if
fastboot_net_abort is called (f.ex. by the first fastboot_tx_print
inside cb_download) before we open download_fd. In that case there
is no poller to check for the timeout.

[...]

> +static void fastboot_handle_type_fastboot(struct fastboot_net *fbn,
> +					  struct fastboot_header header,
> +					  char *fastboot_data,
> +					  unsigned int fastboot_data_len)
> +{
> +	struct fastboot_work *w;
> +
> +	fbn->response_header = header;
> +	fbn->host_waits_since = get_time_ns();
> +	fbn->may_send = fastboot_data_len ? MAY_SEND_ACK : MAY_SEND_MESSAGE;
> +
> +	if (fbn->active_download) {
> +		fbn->last_download_pkt = get_time_ns();
> +
> +		if (!fastboot_data_len && fbn->fastboot.download_bytes
> +					   == fbn->fastboot.download_size) {
> +
> +			fbn->active_download = false;
> +
> +			w = xzalloc(sizeof(*w));
> +			w->fbn = fbn;
> +			w->download_finished = true;
> +
> +			wq_queue_work(&fbn->wq, &w->work);
> +		} else {
> +			fastboot_data_download(fbn, fastboot_data,
> +					       fastboot_data_len);
> +		}
> +		return;
> +	}
> +
> +	if (fastboot_data_len >= FASTBOOT_MAX_CMD_LEN) {

Still off-by-one. Replace >= with >

[...]

> +	case FASTBOOT_INIT:
> +		if (ntohs(header.seq) != fbn->sequence_number) {
> +			fastboot_check_retransmit(fbn, header);
> +			break;
> +		}
> +		fbn->host_addr = net_read_ip(&ip_header->saddr);
> +		fbn->host_port = udp_header->uh_sport;
> +		memcpy(fbn->host_mac, eth_header->et_src, ETH_ALEN);
> +		fastboot_net_abort(fbn);
> +		ret = poller_register(&fbn->poller, "fastboot");
> +		if (ret) {
> +			pr_err("Cannot register poller: %s\n", strerror(-ret));
> +			return;

It is not obvious that a second FASTBOOT_INIT will _not_ cause this
error because fastboot_net_abort unregistered the previous poller.
I would at least add a comment to the fastboot_net_abort(fbn) line.

[...]

> +static void fastboot_poll(struct poller_struct *poller)
> +{
> +	struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
> +					       poller);
> +
> +	if (fbn->active_download && is_timeout(fbn->last_download_pkt, 5 * SECOND)) {

Should pollers prefer is_timeout_non_interruptible over is_timeout?

I can make a new patch set where all issues are fixed, if you don't insist
on doing it yourself.

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 17/21] fastboot net: implement fastboot over UDP

[Sascha Hauer]

Hi Daniel,

On Mon, Jun 29, 2020 at 09:50:51PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
> 
> > +{
> > +	struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
> > +					       poller);
> > +
> > +	if (fbn->active_download && is_timeout(fbn->last_download_pkt, 5 * SECOND)) {
> 
> Should pollers prefer is_timeout_non_interruptible over is_timeout?
> 
> I can make a new patch set where all issues are fixed, if you don't insist
> on doing it yourself.

Should have read this mail up to the end. I was about to tell you that I
won't find time in the next weeks to fix your valid review points, so
indeed I'd be happy if you could do this.

Regards,
 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 17/21] fastboot net: implement fastboot over UDP

[Sascha Hauer]

Hi Daniel,

On Mon, Jun 29, 2020 at 09:50:51PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
> 
> Am 19.06.20 um 09:44 schrieb Sascha Hauer:
> > +struct fastboot_net {
> > +	struct fastboot fastboot;
> > +
> > +	struct net_connection *net_con;
> > +	struct fastboot_header response_header;
> > +	struct poller_struct poller;
> > +	struct work_queue wq;
> > +	u64 host_waits_since;
> > +	u64 last_download_pkt;
> > +	bool sequence_number_seen;
> > +	bool active_download;
> > +	bool reinit;
> > +	bool send_keep_alive;
> > +	enum may_send may_send;
> > +
> > +	IPaddr_t host_addr;
> > +	u16 host_port;
> > +	u8 host_mac[ETH_ALEN];
> > +	u16 sequence_number;
> > +	u16 last_payload_len;
> > +	uchar last_payload[FASTBOOT_MAX_CMD_LEN + sizeof(struct fastboot_header)];
> 
> This is not FASTBOOT_MAX_CMD_LEN. It's the 64 that is strewn around in
> fastboot_tx_print. Adding a new constant FASTBOOT_MAX_MSG_LEN would be
> correct.

There's no check that the packet copied into last_payload has this
maximum size. I switched to storing the packet in an allocated buffer,
with this the define is not needed anymore.

> > +	net_write_ip(&fbn->net_con->ip->daddr, fbn->host_addr);
> > +	fbn->net_con->udp->uh_dport = fbn->host_port;
> > +	net_udp_send(fbn->net_con, fbn->last_payload_len);
> > +
> > +	fbn->may_send = MAY_NOT_SEND;
> 
> You moved that line below net_udp_send. Is there any risk that
> 
> 1. our work queue executes a command which calls fastboot_tx_print
> 2. the net_udp_send caused by that fastboot_tx_print sleeps
> 3. our poller is executed and decides to send a message because
>    may_send is still MAY_SEND_MESSAGE

Ok, changed that.

> > +	fbn->last_download_pkt = get_time_ns();
> > +}
> 
> Although you added that last_download_pkt timeout check to the poller,
> there is still the risk that we will never close download_fd if
> fastboot_net_abort is called (f.ex. by the first fastboot_tx_print
> inside cb_download) before we open download_fd. In that case there
> is no poller to check for the timeout.

I was not able to provoke such a behaviour. It seems that
fastboot_abort() is called often enough that this doesn't happen. In
doubt it happens when the next fastboot session is initiated.

> > +	if (fastboot_data_len >= FASTBOOT_MAX_CMD_LEN) {
> 
> Still off-by-one. Replace >= with >

Ok, fixed.

> > +		ret = poller_register(&fbn->poller, "fastboot");
> > +		if (ret) {
> > +			pr_err("Cannot register poller: %s\n", strerror(-ret));
> > +			return;
> 
> It is not obvious that a second FASTBOOT_INIT will _not_ cause this
> error because fastboot_net_abort unregistered the previous poller.
> I would at least add a comment to the fastboot_net_abort(fbn) line.

Added a comment.

> > +{
> > +	struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
> > +					       poller);
> > +
> > +	if (fbn->active_download && is_timeout(fbn->last_download_pkt, 5 * SECOND)) {
> 
> Should pollers prefer is_timeout_non_interruptible over is_timeout?

Since with the current approach we no longer execute pollers inside of
pollers this shouldn't make a difference.

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