* [PATCH 01/19] Introduce slices
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 02/19] Add workqueues Sascha Hauer
` (17 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 02/19] Add workqueues
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
2020-06-17 8:11 ` [PATCH 01/19] Introduce slices Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 18:30 ` Daniel Glöckner
2020-06-17 8:11 ` [PATCH 03/19] ratp: Switch to workqueues Sascha Hauer
` (16 subsequent siblings)
18 siblings, 1 reply; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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 | 7 +++++++
common/slice.c | 32 +++++++++++++++++++++++++++++++
common/startup.c | 3 +++
common/workqueue.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++
include/slice.h | 5 +++++
include/work.h | 38 ++++++++++++++++++++++++++++++++++++
8 files changed, 140 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..7e8fb50122 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;
@@ -114,11 +116,16 @@ void poller_call(void)
if (poller_active)
return;
+ if (!slice_acquired(&command_slice))
+ wq_do_all_works();
+
+ command_slice_acquire();
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..5fb241e5ce
--- /dev/null
+++ b/common/workqueue.c
@@ -0,0 +1,48 @@
+// 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);
+ }
+}
+
+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..81bbcb7683
--- /dev/null
+++ b/include/work.h
@@ -0,0 +1,38 @@
+#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);
+}
+
+static inline 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);
+ }
+}
+
+void wq_register(struct work_queue *wq);
+void wq_unregister(struct work_queue *wq);
+
+void wq_do_all_works(void);
+
+#endif /* __WORK_H */
--
2.27.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 02/19] Add workqueues
2020-06-17 8:11 ` [PATCH 02/19] Add workqueues Sascha Hauer
@ 2020-06-17 18:30 ` Daniel Glöckner
2020-06-17 19:56 ` Sascha Hauer
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Glöckner @ 2020-06-17 18:30 UTC (permalink / raw)
To: Sascha Hauer, Barebox List
Hello Sascha,
Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> +static inline 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);
> + }
> +}
Why is this function in the header?
So far it is called only from wq_unregister.
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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 02/19] Add workqueues
2020-06-17 18:30 ` Daniel Glöckner
@ 2020-06-17 19:56 ` Sascha Hauer
0 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 19:56 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List
On Wed, Jun 17, 2020 at 08:30:27PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
>
> Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> > +static inline 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);
> > + }
> > +}
>
> Why is this function in the header?
> So far it is called only from wq_unregister.
Right, we can move it there. I think my original intention was that
the user calls wq_cancel_work() before calling wq_unregister().
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 03/19] ratp: Switch to workqueues
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
2020-06-17 8:11 ` [PATCH 01/19] Introduce slices Sascha Hauer
2020-06-17 8:11 ` [PATCH 02/19] Add workqueues Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 04/19] net: Add a slice to struct eth_device Sascha Hauer
` (15 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 04/19] net: Add a slice to struct eth_device
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (2 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 03/19] ratp: Switch to workqueues Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 05/19] net: mdiobus: Add slice Sascha Hauer
` (14 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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, ð_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(ð_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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 05/19] net: mdiobus: Add slice
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (3 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 04/19] net: Add a slice to struct eth_device Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 06/19] usb: Add a slice to usb host controllers Sascha Hauer
` (13 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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 a9fdf44f1a..3f0f31b35c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -16,6 +16,7 @@
#define __PHY_H
#include <driver.h>
+#include <slice.h>
#include <linux/list.h>
#include <linux/ethtool.h>
#include <linux/mii.h>
@@ -116,9 +117,16 @@ struct mii_bus {
struct list_head list;
bool is_multiplexed;
+
+ struct slice slice;
};
#define to_mii_bus(d) container_of(d, struct mii_bus, dev)
+static inline struct slice *mdiobus_slice(struct mii_bus *bus)
+{
+ return &bus->slice;
+}
+
int mdiobus_register(struct mii_bus *bus);
void mdiobus_unregister(struct mii_bus *bus);
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
@@ -134,28 +142,8 @@ struct mii_bus *mdiobus_get_bus(int busnum);
struct mii_bus *of_mdio_find_bus(struct device_node *mdio_bus_np);
-/**
- * mdiobus_read - Convenience function for reading a given MII mgmt register
- * @bus: the mii_bus struct
- * @addr: the phy address
- * @regnum: register number to read
- */
-static inline int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
-{
- return bus->read(bus, addr, regnum);
-}
-
-/**
- * mdiobus_write - Convenience function for writing a given MII mgmt register
- * @bus: the mii_bus struct
- * @addr: the phy address
- * @regnum: register number to write
- * @val: value to write to @regnum
- */
-static inline int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
-{
- return bus->write(bus, addr, regnum, val);
-}
+int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
+int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
/* phy_device: An instance of a PHY
*
@@ -376,11 +364,15 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
int phy_register_fixup_for_id(const char *bus_id,
int (*run)(struct phy_device *));
int phy_scan_fixups(struct phy_device *phydev);
-
int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
u16 data);
+static inline bool phy_acquired(struct phy_device *phydev)
+{
+ return phydev && 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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 06/19] usb: Add a slice to usb host controllers
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (4 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 05/19] net: mdiobus: Add slice Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 07/19] usbnet: Add slice Sascha Hauer
` (12 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 07/19] usbnet: Add slice
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (5 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 06/19] usb: Add a slice to usb host controllers Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 08/19] net: Call net_poll() in a poller Sascha Hauer
` (11 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 08/19] net: Call net_poll() in a poller
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (6 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 07/19] usbnet: Add slice Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 09/19] net: reply to ping requests Sascha Hauer
` (10 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 09/19] net: reply to ping requests
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (7 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 08/19] net: Call net_poll() in a poller Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 10/19] usbnet: Be more friendly in the receive path Sascha Hauer
` (9 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 10/19] usbnet: Be more friendly in the receive path
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (8 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 09/19] net: reply to ping requests Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 11/19] defconfigs: update renamed fastboot options Sascha Hauer
` (8 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 11/19] defconfigs: update renamed fastboot options
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (9 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 10/19] usbnet: Be more friendly in the receive path Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 12/19] globalvar: Add helper for deprecated variable names Sascha Hauer
` (7 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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 | 2 +-
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(+), 7 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 5bf908ee85..fb7872434e 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -66,6 +66,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
@@ -160,7 +161,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_IMX_IPUV3=y
CONFIG_DRIVER_VIDEO_IMX_IPUV3_LVDS=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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 12/19] globalvar: Add helper for deprecated variable names
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (10 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 11/19] defconfigs: update renamed fastboot options Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 13/19] fastboot: rename usbgadget.fastboot_* variables to fastboot.* Sascha Hauer
` (6 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 13/19] fastboot: rename usbgadget.fastboot_* variables to fastboot.*
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (11 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 12/19] globalvar: Add helper for deprecated variable names Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 14/19] fastboot: remove double print Sascha Hauer
` (5 subsequent siblings)
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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 f8df531ff8..a647d8ffd4 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;
@@ -909,14 +911,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 b3e7155efa..88ec478499 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -54,6 +54,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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 14/19] fastboot: remove double print
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (12 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 13/19] fastboot: rename usbgadget.fastboot_* variables to fastboot.* Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 18:24 ` Daniel Glöckner
2020-06-17 8:11 ` [PATCH 15/19] fastboot net: implement fastboot over UDP Sascha Hauer
` (4 subsequent siblings)
18 siblings, 1 reply; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
In fastboot UDP we can't send two messages without waiting for an ack
from the host in between. Do not send two messages directly after each
other in fastboot_download_finished() to make it safe to be called from
the fastboot UDD code.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/fastboot.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/common/fastboot.c b/common/fastboot.c
index a647d8ffd4..f325c6247f 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -340,9 +340,6 @@ void fastboot_download_finished(struct fastboot *fb)
printf("\n");
- fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "Downloading %d bytes finished",
- fb->download_bytes);
-
fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
}
--
2.27.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 14/19] fastboot: remove double print
2020-06-17 8:11 ` [PATCH 14/19] fastboot: remove double print Sascha Hauer
@ 2020-06-17 18:24 ` Daniel Glöckner
2020-06-17 19:54 ` Sascha Hauer
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Glöckner @ 2020-06-17 18:24 UTC (permalink / raw)
To: Sascha Hauer, Barebox List
Hello Sascha,
Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> In fastboot UDP we can't send two messages without waiting for an ack
> from the host in between. Do not send two messages directly after each
> other in fastboot_download_finished() to make it safe to be called from
> the fastboot UDD code.
UDD -> UDP?
I see no reason for this patch. When this function is called, we have just
managed to transfer a file where every data packet was sent only after the
host has received our ack for the previous packet. And now you fear that the
host won't ack our next packet?
The fastboot UDP code in U-Boot was designed to send only a single message
for every executed command because it doesn't have code that waits for a
packet from the host when necessary. I think we should be proud that Barebox
can handle that case.
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 14/19] fastboot: remove double print
2020-06-17 18:24 ` Daniel Glöckner
@ 2020-06-17 19:54 ` Sascha Hauer
2020-06-18 19:08 ` Daniel Glöckner
0 siblings, 1 reply; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 19:54 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List
On Wed, Jun 17, 2020 at 08:24:48PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
>
> Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> > In fastboot UDP we can't send two messages without waiting for an ack
> > from the host in between. Do not send two messages directly after each
> > other in fastboot_download_finished() to make it safe to be called from
> > the fastboot UDD code.
>
> UDD -> UDP?
Yes.
>
> I see no reason for this patch. When this function is called, we have just
> managed to transfer a file where every data packet was sent only after the
> host has received our ack for the previous packet. And now you fear that the
> host won't ack our next packet?
The host indeed acks our next packet. The problem is that we are in
poller context and can't wait for the ack. See fastboot_write_net():
/*
* 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)
return ret;
It actively waits for the ack, but we will never receive it because we
don't run a second poller in the background.
Another option would be to put calling of fastboot_download_finished()
into a workqueue, or just open code it as your original series did.
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 14/19] fastboot: remove double print
2020-06-17 19:54 ` Sascha Hauer
@ 2020-06-18 19:08 ` Daniel Glöckner
2020-06-19 7:01 ` Sascha Hauer
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Glöckner @ 2020-06-18 19:08 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
Am 17.06.20 um 21:54 schrieb Sascha Hauer:
> Another option would be to put calling of fastboot_download_finished()
> into a workqueue, or just open code it as your original series did.
... now I understand that by "open code it" you are referring to calling
net_poll!
I would have tried to run everything from fastboot_poll in a workqueue.
Since worklets are not reordered, splitting fastboot_poll into two worklets
might make sense. The worklet for calling fastboot_download_finished is
queued when fastboot_handle_type_fastboot sets active_download to false and
the worklet for the command execution is queued, as you implemented it, when
a new command arrives.
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 14/19] fastboot: remove double print
2020-06-18 19:08 ` Daniel Glöckner
@ 2020-06-19 7:01 ` Sascha Hauer
0 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-19 7:01 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List
On Thu, Jun 18, 2020 at 09:08:01PM +0200, Daniel Glöckner wrote:
> Am 17.06.20 um 21:54 schrieb Sascha Hauer:
> > Another option would be to put calling of fastboot_download_finished()
> > into a workqueue, or just open code it as your original series did.
>
> ... now I understand that by "open code it" you are referring to calling
> net_poll!
By "open code" I meant that the functionality of
fastboot_download_finished() was duplicated in the fastboot net code,
but this was never the case. It seems my memory tricked me.
>
> I would have tried to run everything from fastboot_poll in a workqueue.
> Since worklets are not reordered, splitting fastboot_poll into two worklets
> might make sense. The worklet for calling fastboot_download_finished is
> queued when fastboot_handle_type_fastboot sets active_download to false and
> the worklet for the command execution is queued, as you implemented it, when
> a new command arrives.
I gave it a try and it was easy enough to implement, so we can drop this
patch.
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 15/19] fastboot net: implement fastboot over UDP
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (13 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 14/19] fastboot: remove double print Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 19:32 ` Daniel Glöckner
2020-06-17 8:11 ` [PATCH 16/19] usb: fastboot: execute commands in command context Sascha Hauer
` (3 subsequent siblings)
18 siblings, 1 reply; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
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 | 515 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 544 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 f325c6247f..195a0f4ffb 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -243,6 +243,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,
@@ -271,6 +272,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;
}
@@ -285,6 +287,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 88ec478499..fcf5a51739 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:
*
@@ -52,6 +54,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..be1619c125
--- /dev/null
+++ b/net/fastboot.c
@@ -0,0 +1,515 @@
+// 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,
+};
+
+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 keep_alive_poller;
+ struct work_queue wq;
+ u64 host_waits_since;
+ bool sequence_number_seen;
+ bool active_download;
+ bool reinit;
+ bool send_keep_alive;
+ bool 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_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) {
+ fbn->fastboot.active = false;
+ fbn->active_download = false;
+ fbn->reinit = true;
+ }
+ 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)
+ 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)
+ return ret;
+
+ 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 = false;
+
+ 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;
+}
+
+/* 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;
+ 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)
+{
+ fbn->response_header = header;
+ fbn->host_waits_since = get_time_ns();
+ fbn->may_send = true;
+
+ if (fbn->active_download) {
+ if (!fastboot_data_len && fbn->fastboot.download_bytes
+ == fbn->fastboot.download_size) {
+ fastboot_download_finished(&fbn->fastboot);
+ } else {
+ fastboot_data_download(fbn, fastboot_data,
+ fastboot_data_len);
+ }
+ } else {
+ if (fastboot_data_len >= FASTBOOT_MAX_CMD_LEN) {
+ fastboot_send(fbn, header, "command too long");
+ } else if (fastboot_data_len) {
+ struct fastboot_work *w;
+
+ 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);
+ }
+ }
+
+ if (!fbn->fastboot.active)
+ fbn->active_download = false;
+}
+
+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);
+
+ /* 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);
+ fbn->reinit = true;
+ if (fbn->active_download) {
+ /*
+ * it is safe to call
+ * fastboot_download_finished here
+ * because reinit is true
+ */
+ fastboot_download_finished(&fbn->fastboot);
+ fbn->active_download = false;
+ }
+ 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;
+
+ 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;
+
+ 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_send_keep_alive(struct poller_struct *poller)
+{
+ struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
+ keep_alive_poller);
+
+ if (!fbn->send_keep_alive)
+ return;
+
+ if (!is_timeout_non_interruptible(fbn->host_waits_since,
+ 30ULL * NSEC_PER_SEC))
+ return;
+
+ if (!fbn->may_send)
+ return;
+
+ fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_INFO, "still busy");
+
+ fbn->host_waits_since = get_time_ns();
+}
+
+void fastboot_net_free(struct fastboot_net *fbn)
+{
+ fastboot_generic_close(&fbn->fastboot);
+ poller_unregister(&fbn->keep_alive_poller);
+ 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));
+ INIT_LIST_HEAD(&fbn->fastboot.variables);
+ 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->keep_alive_poller.func = fastboot_send_keep_alive;
+ ret = poller_register(&fbn->keep_alive_poller,
+ "fastboot-net-keep-alive");
+ if (ret)
+ goto fail_poller2;
+
+ fbn->wq.fn = fastboot_do_work;
+ fbn->wq.cancel = fastboot_work_cancel;
+
+ wq_register(&fbn->wq);
+
+ return fbn;
+
+fail_poller2:
+ net_unregister(fbn->net_con);
+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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 15/19] fastboot net: implement fastboot over UDP
2020-06-17 8:11 ` [PATCH 15/19] fastboot net: implement fastboot over UDP Sascha Hauer
@ 2020-06-17 19:32 ` Daniel Glöckner
2020-06-18 11:59 ` Sascha Hauer
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Glöckner @ 2020-06-17 19:32 UTC (permalink / raw)
To: Sascha Hauer, Barebox List; +Cc: Edmund Henniges
Hello Sascha,
Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> + ret = fastboot_net_wait_may_send(fbn);
> + if (ret)
> + return ret;
Where is the part that aborts the session on timeout?
> +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;
> +}
You didn't base v4 on v3, did you?
If FASTBOOT_INIT is received before active_download is set to true, nobody
will close fb->download_fd.
> +static void fastboot_handle_type_fastboot(struct fastboot_net *fbn,
> + struct fastboot_header header,
> + char *fastboot_data,
> + unsigned int fastboot_data_len)
> +{
> + fbn->response_header = header;
> + fbn->host_waits_since = get_time_ns();
> + fbn->may_send = true;
No MAY_SEND_ACK and MAY_SEND_MESSAGE? So you choose to ignore protocol
violations?
> + if (fbn->active_download) {
> + if (!fastboot_data_len && fbn->fastboot.download_bytes
> + == fbn->fastboot.download_size) {
> + fastboot_download_finished(&fbn->fastboot);
Now I see why you dropped that fastboot_tx_print in the previous patch.
> + } else {
> + fastboot_data_download(fbn, fastboot_data,
> + fastboot_data_len);
> + }
> + } else {
Fastboot does not allow to queue multiple commands. A command must have
finished before the host may send the next one. That's why there was
an "else if (!fbn->command[0])".
> + if (fastboot_data_len >= FASTBOOT_MAX_CMD_LEN) {
Off-by-one error: if (fastboot_data_len > FASTBOOT_MAX_CMD_LEN) {
> + } else if (fastboot_data_len) {
> + struct fastboot_work *w;
> +
> + w = xzalloc(sizeof(*w));
Why can't we embed struct fastboot_work in struct fastboot_net?
As I wrote above it is impossible to queue multiple commands.
> + w->fbn = fbn;
> + memcpy(w->command, fastboot_data, fastboot_data_len);
> + w->command[fastboot_data_len] = 0;
> +
> + wq_queue_work(&fbn->wq, &w->work);
> + }
> + }
> +
> + if (!fbn->fastboot.active)
> + fbn->active_download = false;
These two lines were gone in v3.
> +static void fastboot_send_keep_alive(struct poller_struct *poller)
> +{
> + struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
> + keep_alive_poller);
> +
> + if (!fbn->send_keep_alive)
> + return;
> +
> + if (!is_timeout_non_interruptible(fbn->host_waits_since,
> + 30ULL * NSEC_PER_SEC))
> + return;
> +
> + if (!fbn->may_send)
> + return;
> +
> + fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_INFO, "still busy");
> +
> + fbn->host_waits_since = get_time_ns();
Why do we need this line?
host_waits_since is assigned get_time_ns() when may_send is set to true.
All in all I am not sure I like your simplifications wrt. ack handling,
may_send, and sending multiple messages.
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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 15/19] fastboot net: implement fastboot over UDP
2020-06-17 19:32 ` Daniel Glöckner
@ 2020-06-18 11:59 ` Sascha Hauer
2020-06-18 18:33 ` Daniel Glöckner
0 siblings, 1 reply; 32+ messages in thread
From: Sascha Hauer @ 2020-06-18 11:59 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List, Edmund Henniges
On Wed, Jun 17, 2020 at 09:32:45PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
>
> Am 17.06.20 um 10:11 schrieb Sascha Hauer:
>
> > + ret = fastboot_net_wait_may_send(fbn);
> > + if (ret)
> > + return ret;
>
> Where is the part that aborts the session on timeout?
Ok, will add it (back)
>
> > +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;
> > +}
>
> You didn't base v4 on v3, did you?
No, you sent v3 while I had different changes in my version already.
> If FASTBOOT_INIT is received before active_download is set to true, nobody
> will close fb->download_fd.
Ok, I'll add a fastboot_abort() function like you did in v3.
>
> > +static void fastboot_handle_type_fastboot(struct fastboot_net *fbn,
> > + struct fastboot_header header,
> > + char *fastboot_data,
> > + unsigned int fastboot_data_len)
> > +{
> > + fbn->response_header = header;
> > + fbn->host_waits_since = get_time_ns();
> > + fbn->may_send = true;
>
> No MAY_SEND_ACK and MAY_SEND_MESSAGE? So you choose to ignore protocol
> violations?
Will add it back.
>
> > + if (fbn->active_download) {
> > + if (!fastboot_data_len && fbn->fastboot.download_bytes
> > + == fbn->fastboot.download_size) {
> > + fastboot_download_finished(&fbn->fastboot);
>
> Now I see why you dropped that fastboot_tx_print in the previous patch.
>
> > + } else {
> > + fastboot_data_download(fbn, fastboot_data,
> > + fastboot_data_len);
> > + }
> > + } else {
>
> Fastboot does not allow to queue multiple commands. A command must have
> finished before the host may send the next one. That's why there was
> an "else if (!fbn->command[0])".
Ah, I see. This would be !list_empty(&fbn->wq.work) now.
>
> > + if (fastboot_data_len >= FASTBOOT_MAX_CMD_LEN) {
>
> Off-by-one error: if (fastboot_data_len > FASTBOOT_MAX_CMD_LEN) {
Ok.
>
> > + } else if (fastboot_data_len) {
> > + struct fastboot_work *w;
> > +
> > + w = xzalloc(sizeof(*w));
>
> Why can't we embed struct fastboot_work in struct fastboot_net?
> As I wrote above it is impossible to queue multiple commands.
We could do that, but I prefer to keep it allocated. It makes the
lifetime of the work item clearer.
>
> > + w->fbn = fbn;
> > + memcpy(w->command, fastboot_data, fastboot_data_len);
> > + w->command[fastboot_data_len] = 0;
> > +
> > + wq_queue_work(&fbn->wq, &w->work);
> > + }
> > + }
> > +
> > + if (!fbn->fastboot.active)
> > + fbn->active_download = false;
>
> These two lines were gone in v3.
Ok, will drop them.
>
> > +static void fastboot_send_keep_alive(struct poller_struct *poller)
> > +{
> > + struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
> > + keep_alive_poller);
> > +
> > + if (!fbn->send_keep_alive)
> > + return;
> > +
> > + if (!is_timeout_non_interruptible(fbn->host_waits_since,
> > + 30ULL * NSEC_PER_SEC))
> > + return;
> > +
> > + if (!fbn->may_send)
> > + return;
> > +
> > + fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_INFO, "still busy");
> > +
> > + fbn->host_waits_since = get_time_ns();
>
> Why do we need this line?
> host_waits_since is assigned get_time_ns() when may_send is set to true.
It is needed to reinitialize the timeout when we sent the message once.
Without this we would send the "still busy" message every few ms after
we've sent it for the first time.
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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 15/19] fastboot net: implement fastboot over UDP
2020-06-18 11:59 ` Sascha Hauer
@ 2020-06-18 18:33 ` Daniel Glöckner
2020-06-19 7:38 ` Sascha Hauer
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Glöckner @ 2020-06-18 18:33 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List, Edmund Henniges
Hello Sascha,
Am 18.06.20 um 13:59 schrieb Sascha Hauer:
> On Wed, Jun 17, 2020 at 09:32:45PM +0200, Daniel Glöckner wrote:
>>> +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;
>>> +}
>>
>> You didn't base v4 on v3, did you?
>
> No, you sent v3 while I had different changes in my version already.
>
>> If FASTBOOT_INIT is received before active_download is set to true, nobody
>> will close fb->download_fd.
>
> Ok, I'll add a fastboot_abort() function like you did in v3.
What I was referring to are the three additional lines in v3 that call
fastboot_download_finished here instead of setting active_download to true
when reinit is already true.
>>> +static void fastboot_send_keep_alive(struct poller_struct *poller)
>>> +{
>>> + struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
>>> + keep_alive_poller);
>>> +
>>> + if (!fbn->send_keep_alive)
>>> + return;
>>> +
>>> + if (!is_timeout_non_interruptible(fbn->host_waits_since,
>>> + 30ULL * NSEC_PER_SEC))
>>> + return;
>>> +
>>> + if (!fbn->may_send)
>>> + return;
>>> +
>>> + fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_INFO, "still busy");
>>> +
>>> + fbn->host_waits_since = get_time_ns();
>>
>> Why do we need this line?
>> host_waits_since is assigned get_time_ns() when may_send is set to true.
>
> It is needed to reinitialize the timeout when we sent the message once.
> Without this we would send the "still busy" message every few ms after
> we've sent it for the first time.
No, because sending a message sets may_send to false and it stays false
until we update host_waits_since in fastboot_handle_type_fastboot.
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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 15/19] fastboot net: implement fastboot over UDP
2020-06-18 18:33 ` Daniel Glöckner
@ 2020-06-19 7:38 ` Sascha Hauer
0 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-19 7:38 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List, Edmund Henniges
On Thu, Jun 18, 2020 at 08:33:03PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
>
> Am 18.06.20 um 13:59 schrieb Sascha Hauer:
> > On Wed, Jun 17, 2020 at 09:32:45PM +0200, Daniel Glöckner wrote:
> >>> +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;
> >>> +}
> >>
> >> You didn't base v4 on v3, did you?
> >
> > No, you sent v3 while I had different changes in my version already.
> >
> >> If FASTBOOT_INIT is received before active_download is set to true, nobody
> >> will close fb->download_fd.
> >
> > Ok, I'll add a fastboot_abort() function like you did in v3.
>
> What I was referring to are the three additional lines in v3 that call
> fastboot_download_finished here instead of setting active_download to true
> when reinit is already true.
>
> >>> +static void fastboot_send_keep_alive(struct poller_struct *poller)
> >>> +{
> >>> + struct fastboot_net *fbn = container_of(poller, struct fastboot_net,
> >>> + keep_alive_poller);
> >>> +
> >>> + if (!fbn->send_keep_alive)
> >>> + return;
> >>> +
> >>> + if (!is_timeout_non_interruptible(fbn->host_waits_since,
> >>> + 30ULL * NSEC_PER_SEC))
> >>> + return;
> >>> +
> >>> + if (!fbn->may_send)
> >>> + return;
> >>> +
> >>> + fastboot_tx_print(&fbn->fastboot, FASTBOOT_MSG_INFO, "still busy");
> >>> +
> >>> + fbn->host_waits_since = get_time_ns();
> >>
> >> Why do we need this line?
> >> host_waits_since is assigned get_time_ns() when may_send is set to true.
> >
> > It is needed to reinitialize the timeout when we sent the message once.
> > Without this we would send the "still busy" message every few ms after
> > we've sent it for the first time.
>
> No, because sending a message sets may_send to false and it stays false
> until we update host_waits_since in fastboot_handle_type_fastboot.
You are right. I introduced this when I had a version that flooded the
network with keepalive packets after 30s. Apparently this no longer
happens, so I'll remove this line.
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 16/19] usb: fastboot: execute commands in command context
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (14 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 15/19] fastboot net: implement fastboot over UDP Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 19:40 ` Daniel Glöckner
2020-06-17 8:11 ` [PATCH 17/19] Add WARN_ONCE() macro Sascha Hauer
` (2 subsequent siblings)
18 siblings, 1 reply; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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 | 49 ++++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index f8a9c32530..d3a3e43757 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);
}
@@ -430,16 +465,16 @@ 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;
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;
+
+ memcpy(w->command, req->buf, req->actual);
+
+ 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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 16/19] usb: fastboot: execute commands in command context
2020-06-17 8:11 ` [PATCH 16/19] usb: fastboot: execute commands in command context Sascha Hauer
@ 2020-06-17 19:40 ` Daniel Glöckner
2020-06-18 7:26 ` Sascha Hauer
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Glöckner @ 2020-06-17 19:40 UTC (permalink / raw)
To: Sascha Hauer, Barebox List
Hello Sascha,
Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> - *(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;
> +
> + memcpy(w->command, req->buf, req->actual);
> +
> + wq_queue_work(&f_fb->wq, &w->work);
Do we need to check if req->actual < sizeof(w->command)?
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 16/19] usb: fastboot: execute commands in command context
2020-06-17 19:40 ` Daniel Glöckner
@ 2020-06-18 7:26 ` Sascha Hauer
0 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-18 7:26 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List
On Wed, Jun 17, 2020 at 09:40:56PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
>
> Am 17.06.20 um 10:11 schrieb Sascha Hauer:
> > - *(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;
> > +
> > + memcpy(w->command, req->buf, req->actual);
> > +
> > + wq_queue_work(&f_fb->wq, &w->work);
>
> Do we need to check if req->actual < sizeof(w->command)?
Yes, will add.
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 17/19] Add WARN_ONCE() macro
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (15 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 16/19] usb: fastboot: execute commands in command context Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 18/19] fs: Warn when filesystem operations are called from a poller Sascha Hauer
2020-06-17 8:11 ` [PATCH 19/19] Documentation: Add document for parallel execution in barebox Sascha Hauer
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 18/19] fs: Warn when filesystem operations are called from a poller
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (16 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 17/19] Add WARN_ONCE() macro Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
2020-06-17 8:11 ` [PATCH 19/19] Documentation: Add document for parallel execution in barebox Sascha Hauer
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
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 7e8fb50122..d25fc9a294 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
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 19/19] Documentation: Add document for parallel execution in barebox
2020-06-17 8:11 [PATCH v4 00/19] Slices and fastboot over UDP Sascha Hauer
` (17 preceding siblings ...)
2020-06-17 8:11 ` [PATCH 18/19] fs: Warn when filesystem operations are called from a poller Sascha Hauer
@ 2020-06-17 8:11 ` Sascha Hauer
18 siblings, 0 replies; 32+ messages in thread
From: Sascha Hauer @ 2020-06-17 8:11 UTC (permalink / raw)
To: Barebox List; +Cc: Daniel Glöckner
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Documentation/devel/devel.rst | 14 ++++
Documentation/devel/parallel-execution.rst | 92 ++++++++++++++++++++++
Documentation/index.rst | 1 +
3 files changed, 107 insertions(+)
create mode 100644 Documentation/devel/devel.rst
create mode 100644 Documentation/devel/parallel-execution.rst
diff --git a/Documentation/devel/devel.rst b/Documentation/devel/devel.rst
new file mode 100644
index 0000000000..041545a60d
--- /dev/null
+++ b/Documentation/devel/devel.rst
@@ -0,0 +1,14 @@
+.. highlight:: console
+
+barebox programming
+===================
+
+Contents:
+
+.. toctree::
+ :maxdepth: 2
+
+ parallel-execution
+
+* :ref:`search`
+* :ref:`genindex`
diff --git a/Documentation/devel/parallel-execution.rst b/Documentation/devel/parallel-execution.rst
new file mode 100644
index 0000000000..2a01d25276
--- /dev/null
+++ b/Documentation/devel/parallel-execution.rst
@@ -0,0 +1,92 @@
+Parallel execution in barebox
+=============================
+
+barebox is single threaded only 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 some techniques described below.
+
+Pollers
+-------
+
+Pollers are a way in barebox to frequently execute code in the background.
+barebox is single threaded only, 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 also other reasons loops polling
+for hardware should always include a timeout which is best implemented with
+``is_timeout()``. Another thing to take care about is that pollers can be
+executed anywhere in the middle of other device accesses. Care must be taken
+that a poller doesn't access a device from which itself is called from. 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.
+
+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.
+
+Workqueues
+----------
+
+Sometimes pollers wish to run code that can't be directly executed from
+pollers. For this case workqueues provide a way to asynchronously execute this
+code in a context where this is allowed. 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 test if a device is currently busy and thus may not be
+called into currently. Pollers wanting to access a device must call
+``slice_acquired()`` 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. Devices providing a slice must call ``slice_acquire()`` 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_acquired()`` on the network device
+will return ``true`` when the USB host controller is busy.
+
+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 sluggish to key presses then probably pollers
+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. Workqueues help to run longer running code,
+but during the time 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
+some places 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 currently. Sooner or later the poller toggling the LED will do
+this right in the middle of an unrelated I2C transfer.
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
^ permalink raw reply [flat|nested] 32+ messages in thread