* [PATCH 00/14] usb: gadget: refactor to allow easier extension
@ 2021-04-12 22:34 Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 01/14] show_progress: add system wide progress stage notifier Ahmad Fatoum
` (13 more replies)
0 siblings, 14 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox
There's some duplication between DFU and Fastboot and incoming USB
mass storage support as well as custom protocols that might be
implemented by board vendors could benefit from some consolidation.
This is added here in the form of machine partitions, which is just
a way to have a machine-global file_list, that can be used as fallback
for fastboot, DFU, mass storage gadget, ... etc.
Some commits are not directly related and happened along the way.
Let me know if I should split up the series.
Cheers,
Ahmad Fatoum (14):
show_progress: add system wide progress stage notifier
common: console: add log_dprint to print to file descriptor
string: implement strstarts along with strends
vsprintf: introduce %m shorthand for "%s", strerror(errno)
param: introduce file-list parameter type
common: add generic machine partitions interface
fastboot: handle ill-named partitions gracefully
usb: gadget: dfu: change status message to info log level
usbgadget: autostart: fix indeterminism around usbgadget.autostart
usbgadget: allow DFU and Fastboot functions to coexist
fastboot/dfu: use machine partitions as fall back
bbu: add function to directly add handlers into file_list
file_list: add file_list_detect_all()
common: make FILE_LIST feature unconditional
common/Kconfig | 15 ++++--
common/Makefile | 3 +-
common/bbu.c | 24 ++++++----
common/console_common.c | 15 ++++++
common/fastboot.c | 28 ++++--------
common/file-list.c | 82 +++++++++++++++++++++++++++++++++
common/machine-partitions.c | 44 ++++++++++++++++++
common/usbgadget.c | 81 ++++++++++++++++++---------------
drivers/usb/gadget/Kconfig | 2 -
drivers/usb/gadget/dfu.c | 3 +-
drivers/usb/gadget/multi.c | 12 +++++
include/bbu.h | 10 +++-
include/fastboot.h | 6 +--
include/file-list.h | 12 +++++
include/linux/string.h | 10 ++++
include/machine-partitions.h | 40 ++++++++++++++++
include/of.h | 5 ++
include/param.h | 15 ++++++
include/printk.h | 1 +
include/progress.h | 43 ++++++++++++++++++
include/string.h | 1 +
include/stringlist.h | 1 +
include/usb/gadget-multi.h | 1 +
lib/Kconfig | 6 +++
lib/parameter.c | 88 ++++++++++++++++++++++++++++++++++++
lib/show_progress.c | 28 ++++++++++++
lib/string.c | 9 ++++
lib/stringlist.c | 30 ++++++++++++
lib/vsprintf.c | 13 ++++++
net/Kconfig | 1 -
net/fastboot.c | 4 +-
31 files changed, 552 insertions(+), 81 deletions(-)
create mode 100644 common/machine-partitions.c
create mode 100644 include/machine-partitions.h
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 01/14] show_progress: add system wide progress stage notifier
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-15 7:29 ` Sascha Hauer
2021-04-12 22:34 ` [PATCH 02/14] common: console: add log_dprint to print to file descriptor Ahmad Fatoum
` (12 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Use case is e.g. board code that wants to register a client to light
status LEDs to indicate system state when no serial output is available.
This functionality doesn't increase code size due to linker GC when
CONFIG_PROGRESS_NOTIFIER is disabled.
There is a generic progress notifier provided that just logs the
status. This could be shared with the booted kernel via pstore or
the log as a whole written to a system setup USB drive.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
include/progress.h | 43 +++++++++++++++++++++++++++++++++++++++++++
lib/Kconfig | 6 ++++++
lib/show_progress.c | 28 ++++++++++++++++++++++++++++
3 files changed, 77 insertions(+)
diff --git a/include/progress.h b/include/progress.h
index 50b15fb12b4c..7230bd3a9bd5 100644
--- a/include/progress.h
+++ b/include/progress.h
@@ -3,6 +3,8 @@
#define __PROGRSS_H
#include <linux/types.h>
+#include <notifier.h>
+#include <errno.h>
/* Initialize a progress bar. If max > 0 a one line progress
* bar is printed where 'max' corresponds to 100%. If max == 0
@@ -15,4 +17,45 @@ void init_progression_bar(loff_t max);
*/
void show_progress(loff_t now);
+extern struct notifier_head progress_notifier;
+
+enum progress_stage {
+ PROGRESS_UNSPECIFIED = 0,
+ PROGRESS_UPDATING,
+ PROGRESS_UPDATE_SUCCESS,
+ PROGRESS_UPDATE_FAIL,
+};
+
+/*
+ * Notifier list for code which wants to be called at progress
+ * This could use by board code to e.g. flash a LED during updates
+ */
+extern struct notifier_head progress_notifier_list;
+
+/* generic client that just logs the state */
+extern struct notifier_block progress_log_client;
+
+static inline int progress_register_client(struct notifier_block *nb)
+{
+ if (!IS_ENABLED(CONFIG_PROGRESS_NOTIFIER))
+ return -ENOSYS;
+ return notifier_chain_register(&progress_notifier_list, nb);
+}
+
+static inline int progress_unregister_client(struct notifier_block *nb)
+{
+ if (!IS_ENABLED(CONFIG_PROGRESS_NOTIFIER))
+ return -ENOSYS;
+ return notifier_chain_unregister(&progress_notifier_list, nb);
+}
+
+static inline int progress_notifier_call_chain(enum progress_stage stage, const char *prefix)
+{
+ if (!IS_ENABLED(CONFIG_PROGRESS_NOTIFIER))
+ return -ENOSYS;
+
+ /* clients should not modify the prefix */
+ return notifier_call_chain(&progress_notifier_list, stage, (char *)(prefix ?: ""));
+}
+
#endif /* __PROGRSS_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index e5831ecdb9a7..922710e106b3 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -154,6 +154,12 @@ source "lib/logo/Kconfig"
source "lib/bootstrap/Kconfig"
+config PROGRESS_NOTIFIER
+ bool
+ help
+ This is selected by boards that register a notifier to visualize
+ progress, like blinking a LED during an update.
+
config PRINTF_UUID
bool
diff --git a/lib/show_progress.c b/lib/show_progress.c
index 1be06ea7806e..1b624bcb9af8 100644
--- a/lib/show_progress.c
+++ b/lib/show_progress.c
@@ -57,3 +57,31 @@ void init_progression_bar(loff_t max)
else
printf("\t");
}
+
+NOTIFIER_HEAD(progress_notifier_list);
+
+static int progress_logger(struct notifier_block *r, unsigned long stage, void *_prefix)
+{
+ const char *prefix = _prefix;
+
+ switch ((enum progress_stage)stage) {
+ case PROGRESS_UPDATING:
+ pr_info("%sSoftware update in progress\n", prefix);
+ break;
+ case PROGRESS_UPDATE_SUCCESS:
+ pr_info("%sSoftware update finished successfully\n", prefix);
+ break;
+ case PROGRESS_UPDATE_FAIL:
+ pr_info("%sSoftware update failed\n", prefix);
+ break;
+ case PROGRESS_UNSPECIFIED:
+ /* default state. This should not be reached */
+ break;
+ }
+
+ return 0;
+}
+
+struct notifier_block progress_log_client = {
+ .notifier_call = progress_logger
+};
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/14] common: console: add log_dprint to print to file descriptor
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 01/14] show_progress: add system wide progress stage notifier Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-15 7:25 ` Sascha Hauer
2021-04-12 22:34 ` [PATCH 03/14] string: implement strstarts along with strends Ahmad Fatoum
` (11 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
It can be useful to dump the log into the file, e.g. when doing an
update from a USB flash drive with no serial peer attached. Board code
could use log_dprint to dump the log onto the drive. Add a function
to facilitate this.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/console_common.c | 15 +++++++++++++++
include/printk.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/common/console_common.c b/common/console_common.c
index 3e0741572398..77ad4728fbfd 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -182,6 +182,21 @@ static int console_common_init(void)
}
device_initcall(console_common_init);
+int log_dprint(int fd)
+{
+ struct log_entry *log;
+ int nbytes = 0;
+
+ list_for_each_entry(log, &barebox_logbuf, list) {
+ int ret = dputs(fd, log->msg);
+ if (ret < 0)
+ return ret;
+ nbytes += ret;
+ }
+
+ return nbytes;
+}
+
void log_print(unsigned flags, unsigned levels)
{
struct log_entry *log;
diff --git a/include/printk.h b/include/printk.h
index 94a25ec9ebac..798acfdbf188 100644
--- a/include/printk.h
+++ b/include/printk.h
@@ -141,6 +141,7 @@ extern void log_clean(unsigned int limit);
#define BAREBOX_LOG_PRINT_ALERT BIT(1)
#define BAREBOX_LOG_PRINT_EMERG BIT(0)
+int log_dprint(int fd);
void log_print(unsigned flags, unsigned levels);
struct va_format {
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 03/14] string: implement strstarts along with strends
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 01/14] show_progress: add system wide progress stage notifier Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 02/14] common: console: add log_dprint to print to file descriptor Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 04/14] vsprintf: introduce %m shorthand for "%s", strerror(errno) Ahmad Fatoum
` (10 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Both can be useful when parsing file paths. They are added to different
files, because only one of them is available in upstream
<linux/string.h>. The other we add to <string.h>, which contains more
barebox-specific string functions.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
include/linux/string.h | 10 ++++++++++
include/string.h | 1 +
lib/string.c | 9 +++++++++
3 files changed, 20 insertions(+)
diff --git a/include/linux/string.h b/include/linux/string.h
index 58e2a4d2ea66..55bc905c0e6b 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -136,4 +136,14 @@ extern int kstrtobool(const char *s, bool *res);
int match_string(const char * const *array, size_t n, const char *string);
+/**
+ * strstarts - does @str start with @prefix?
+ * @str: string to examine
+ * @prefix: prefix to look for.
+ */
+static inline bool strstarts(const char *str, const char *prefix)
+{
+ return strncmp(str, prefix, strlen(prefix)) == 0;
+}
+
#endif /* _LINUX_STRING_H_ */
diff --git a/include/string.h b/include/string.h
index ef0b5e199eea..d423bee6fba5 100644
--- a/include/string.h
+++ b/include/string.h
@@ -7,6 +7,7 @@
int strtobool(const char *str, int *val);
char *strsep_unescaped(char **, const char *);
char *stpcpy(char *dest, const char *src);
+bool strends(const char *str, const char *postfix);
void *__default_memset(void *, int, __kernel_size_t);
void *__nokasan_default_memset(void *, int, __kernel_size_t);
diff --git a/lib/string.c b/lib/string.c
index dbb66fe4d2cc..bad186586fac 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -866,6 +866,15 @@ int strtobool(const char *str, int *val)
}
EXPORT_SYMBOL(strtobool);
+bool strends(const char *str, const char *postfix)
+{
+ if (strlen(str) < strlen(postfix))
+ return false;
+
+ return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
+}
+EXPORT_SYMBOL(strends);
+
/**
* match_string - matches given string in an array
* @array: array of strings
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 04/14] vsprintf: introduce %m shorthand for "%s", strerror(errno)
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (2 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 03/14] string: implement strstarts along with strends Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 05/14] param: introduce file-list parameter type Ahmad Fatoum
` (9 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
We have many places across the code base that use either do
printf("%s..", errno_str()) or printf("%s..", strerror(errno).
We already have %pe support for printing arbitrary errors and it's not
much work to add %m to print errno as well. Do so.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
lib/vsprintf.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1d82adc73368..237aab0c02a1 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -373,12 +373,21 @@ static char *pointer(const char *fmt, char *buf, const char *end, const void *pt
return raw_pointer(buf, end, ptr, field_width, precision, flags);
}
+static char *errno_string(char *buf, const char *end, int field_width, int precision, int flags)
+{
+ return string(buf, end, strerror(errno), field_width, precision, flags);
+}
#else
static char *pointer(const char *fmt, char *buf, const char *end, const void *ptr,
int field_width, int precision, int flags)
{
return raw_pointer(buf, end, ptr, field_width, precision, flags);
}
+
+static char *errno_string(char *buf, const char *end, int field_width, int precision, int flags)
+{
+ return buf;
+}
#endif
/**
@@ -569,6 +578,10 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
case 'u':
break;
+ case 'm':
+ str = errno_string(str, end, field_width, precision, flags);
+ continue;
+
default:
if (str < end)
*str = '%';
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 05/14] param: introduce file-list parameter type
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (3 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 04/14] vsprintf: introduce %m shorthand for "%s", strerror(errno) Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 06/14] common: add generic machine partitions interface Ahmad Fatoum
` (8 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
DFU, fastboot and incoming mass storage support all use file lists as
input, but individually check syntax correctness only on use.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/file-list.c | 47 +++++++++++++++++++++++
include/file-list.h | 3 ++
include/param.h | 15 ++++++++
include/stringlist.h | 1 +
lib/parameter.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
lib/stringlist.c | 30 +++++++++++++++
6 files changed, 184 insertions(+)
diff --git a/common/file-list.c b/common/file-list.c
index cd52b5e045de..924903cef7dc 100644
--- a/common/file-list.c
+++ b/common/file-list.c
@@ -6,6 +6,7 @@
#include <malloc.h>
#include <fs.h>
#include <file-list.h>
+#include <stringlist.h>
#include <linux/err.h>
#define PARSE_DEVICE 0
@@ -109,6 +110,25 @@ static int file_list_parse_one(struct file_list *files, const char *partstr, con
return file_list_add_entry(files, name, filename, flags);
}
+static const char *flags_to_str(int flags)
+{
+ static char str[sizeof "srcu"];
+ char *s = str;;
+
+ if (flags & FILE_LIST_FLAG_SAFE)
+ *s++ = 's';
+ if (flags & FILE_LIST_FLAG_READBACK)
+ *s++ = 'r';
+ if (flags & FILE_LIST_FLAG_CREATE)
+ *s++ = 'c';
+ if (flags & FILE_LIST_FLAG_UBI)
+ *s++ = 'u';
+
+ *s = '\0';
+
+ return str;
+}
+
struct file_list *file_list_parse(const char *str)
{
struct file_list *files;
@@ -149,3 +169,30 @@ void file_list_free(struct file_list *files)
free(files);
}
+
+char *file_list_to_str(const struct file_list *files)
+{
+ struct file_list_entry *entry;
+ struct string_list sl;
+ char *str;
+
+ if (!files)
+ return strdup("");
+
+ string_list_init(&sl);
+
+ list_for_each_entry(entry, &files->list, list) {
+ int ret = string_list_add_asprintf(&sl, "%s(%s)%s", entry->filename, entry->name,
+ flags_to_str(entry->flags));
+ if (ret) {
+ str = ERR_PTR(ret);
+ goto out;
+ }
+ }
+
+ str = string_list_join(&sl, ",");
+out:
+ string_list_free(&sl);
+
+ return str;
+}
diff --git a/include/file-list.h b/include/file-list.h
index 9a9edfa378f3..7264a3e2c628 100644
--- a/include/file-list.h
+++ b/include/file-list.h
@@ -2,6 +2,8 @@
#ifndef __FILE_LIST
#define __FILE_LIST
+#include <linux/list.h>
+
#define FILE_LIST_FLAG_SAFE (1 << 0)
#define FILE_LIST_FLAG_READBACK (1 << 1)
#define FILE_LIST_FLAG_CREATE (1 << 2)
@@ -20,6 +22,7 @@ struct file_list {
};
struct file_list *file_list_parse(const char *str);
+char *file_list_to_str(const struct file_list *files);
void file_list_free(struct file_list *);
int file_list_add_entry(struct file_list *files, const char *name, const char *filename,
diff --git a/include/param.h b/include/param.h
index 6aca1b481d92..4835be4d2518 100644
--- a/include/param.h
+++ b/include/param.h
@@ -10,6 +10,7 @@
#define PARAM_GLOBALVAR_UNQUALIFIED (1 << 1)
struct device_d;
+struct file_list;
typedef uint32_t IPaddr_t;
enum param_type {
@@ -23,6 +24,7 @@ enum param_type {
PARAM_TYPE_BITMASK,
PARAM_TYPE_IPV4,
PARAM_TYPE_MAC,
+ PARAM_TYPE_FILE_LIST,
};
struct param_d {
@@ -89,6 +91,11 @@ struct param_d *dev_add_param_mac(struct device_d *dev, const char *name,
int (*get)(struct param_d *p, void *priv),
u8 *mac, void *priv);
+struct param_d *dev_add_param_file_list(struct device_d *dev, const char *name,
+ int (*set)(struct param_d *p, void *priv),
+ int (*get)(struct param_d *p, void *priv),
+ struct file_list **file_list, void *priv);
+
struct param_d *dev_add_param_fixed(struct device_d *dev, const char *name, const char *value);
void dev_remove_param(struct param_d *p);
@@ -185,6 +192,14 @@ static inline struct param_d *dev_add_param_mac(struct device_d *dev, const char
return NULL;
}
+static inline struct param_d *dev_add_param_file_list(struct device_d *dev, const char *name,
+ int (*set)(struct param_d *p, void *priv),
+ int (*get)(struct param_d *p, void *priv),
+ struct file_list **file_list, void *priv)
+{
+ return NULL;
+}
+
static inline struct param_d *dev_add_param_fixed(struct device_d *dev, const char *name,
const char *value)
{
diff --git a/include/stringlist.h b/include/stringlist.h
index 5cd452ca5fa4..094dc7759e76 100644
--- a/include/stringlist.h
+++ b/include/stringlist.h
@@ -13,6 +13,7 @@ int string_list_add(struct string_list *sl, const char *str);
int string_list_add_asprintf(struct string_list *sl, const char *fmt, ...);
int string_list_add_sorted(struct string_list *sl, const char *str);
int string_list_contains(struct string_list *sl, const char *str);
+char *string_list_join(const struct string_list *sl, const char *joinstr);
void string_list_print_by_column(struct string_list *sl);
static inline void string_list_init(struct string_list *sl)
diff --git a/lib/parameter.c b/lib/parameter.c
index f57b7d07fd3a..1f768ab65ede 100644
--- a/lib/parameter.c
+++ b/lib/parameter.c
@@ -27,6 +27,8 @@
#include <string.h>
#include <globalvar.h>
#include <linux/err.h>
+#include <file-list.h>
+#include <stringlist.h>
static const char *param_type_string[] = {
[PARAM_TYPE_STRING] = "string",
@@ -39,6 +41,7 @@ static const char *param_type_string[] = {
[PARAM_TYPE_BITMASK] = "bitmask",
[PARAM_TYPE_IPV4] = "ipv4",
[PARAM_TYPE_MAC] = "MAC",
+ [PARAM_TYPE_FILE_LIST] = "file-list",
};
const char *get_param_type(struct param_d *param)
@@ -909,6 +912,91 @@ struct param_d *dev_add_param_mac(struct device_d *dev, const char *name,
return &pm->param;
}
+struct param_file_list {
+ struct param_d param;
+ struct file_list **file_list;
+ char *file_list_str;
+ int (*set)(struct param_d *p, void *priv);
+ int (*get)(struct param_d *p, void *priv);
+};
+
+static inline struct param_file_list *to_param_file_list(struct param_d *p)
+{
+ return container_of(p, struct param_file_list, param);
+}
+
+static int param_file_list_set(struct device_d *dev, struct param_d *p, const char *val)
+{
+ struct param_file_list *pfl = to_param_file_list(p);
+ struct file_list *file_list_save = *pfl->file_list;
+ int ret;
+
+ if (!val)
+ val = "";
+
+ *pfl->file_list = file_list_parse(val);
+ if (IS_ERR(*pfl->file_list)) {
+ ret = PTR_ERR(*pfl->file_list);
+ goto out;
+ }
+
+ if (pfl->set) {
+ ret = pfl->set(p, p->driver_priv);
+ if (ret) {
+ file_list_free(*pfl->file_list);
+ goto out;
+ }
+ }
+
+ return 0;
+out:
+ *pfl->file_list = file_list_save;
+
+ return ret;
+}
+
+static const char *param_file_list_get(struct device_d *dev, struct param_d *p)
+{
+ struct param_file_list *pfl = to_param_file_list(p);
+ int ret;
+
+ if (pfl->get) {
+ ret = pfl->get(p, p->driver_priv);
+ if (ret)
+ return NULL;
+ }
+
+ free(p->value);
+ p->value = file_list_to_str(*pfl->file_list);
+ return p->value;
+}
+
+struct param_d *dev_add_param_file_list(struct device_d *dev, const char *name,
+ int (*set)(struct param_d *p, void *priv),
+ int (*get)(struct param_d *p, void *priv),
+ struct file_list **file_list, void *priv)
+{
+ struct param_file_list *pfl;
+ int ret;
+
+ pfl = xzalloc(sizeof(*pfl));
+ pfl->file_list = file_list;
+ pfl->set = set;
+ pfl->get = get;
+ pfl->param.driver_priv = priv;
+ pfl->param.type = PARAM_TYPE_FILE_LIST;
+
+ ret = __dev_add_param(&pfl->param, dev, name,
+ param_file_list_set, param_file_list_get, 0);
+ if (ret) {
+ free(pfl);
+ return ERR_PTR(ret);
+ }
+
+ return &pfl->param;
+}
+
+
/**
* dev_remove_param - remove a parameter from a device and free its
* memory
diff --git a/lib/stringlist.c b/lib/stringlist.c
index 8e92c1b207c5..d67da5a1d8f5 100644
--- a/lib/stringlist.c
+++ b/lib/stringlist.c
@@ -2,6 +2,7 @@
#include <xfuncs.h>
#include <malloc.h>
#include <errno.h>
+#include <string.h>
#include <stringlist.h>
static int string_list_compare(struct list_head *a, struct list_head *b)
@@ -72,6 +73,35 @@ int string_list_contains(struct string_list *sl, const char *str)
return 0;
}
+char *string_list_join(const struct string_list *sl, const char *joinstr)
+{
+ struct string_list *entry;
+ size_t len = 0;
+ size_t joinstr_len = strlen(joinstr);
+ char *str, *next;
+
+ string_list_for_each_entry(entry, sl)
+ len += strlen(entry->str) + joinstr_len;
+
+ if (len == 0)
+ return strdup("");
+
+ str = malloc(len + 1);
+ if (!str)
+ return NULL;
+
+ next = str;
+
+ string_list_for_each_entry(entry, sl) {
+ next = stpcpy(next, entry->str);
+ next = stpcpy(next, joinstr);
+ }
+
+ next[-joinstr_len] = '\0';
+
+ return str;
+}
+
void string_list_print_by_column(struct string_list *sl)
{
int len = 0, num, i;
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 06/14] common: add generic machine partitions interface
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (4 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 05/14] param: introduce file-list parameter type Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-14 10:08 ` Sascha Hauer
2021-04-12 22:34 ` [PATCH 07/14] fastboot: handle ill-named partitions gracefully Ahmad Fatoum
` (7 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Both Fastboot and DFU have their own global variables that allow
specifying the partitions that can be flashed via the environment.
With the upcoming addition of the USB mass storage gadget, we will need
some way to define the partitions there as well.
Instead of adding yet another way download method-specific variable,
add a generic machine.partitions variable that can be specified on a
per-board basis and can be used for all methods.
Existing variables will still remain for backwards-compatibility, but
when unset, it should fall back to this new parameter. This is done
if the follow-up patches.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/Kconfig | 11 +++++++++
common/Makefile | 1 +
common/file-list.c | 18 +++++++++++++++
common/machine-partitions.c | 44 ++++++++++++++++++++++++++++++++++++
include/file-list.h | 2 ++
include/machine-partitions.h | 40 ++++++++++++++++++++++++++++++++
include/of.h | 5 ++++
7 files changed, 121 insertions(+)
create mode 100644 common/machine-partitions.c
create mode 100644 include/machine-partitions.h
diff --git a/common/Kconfig b/common/Kconfig
index 2170f985bebc..f9803dbb5219 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -696,6 +696,17 @@ config FLEXIBLE_BOOTARGS
config BAREBOX_UPDATE
bool "In-system barebox update infrastructure"
+config MACHINE_PARTITIONS
+ bool "Generic machine partitions support"
+ depends on OFTREE
+ help
+ Machine partitions are a generic way for boards to specify the
+ partitions of the machine that should be exported for flashing.
+ Board drivers setting this directly will select this option
+ automatically.
+ Say y here if this should be configurable over the
+ machine.partitions device parameter.
+
config IMD
select CRC32
bool "barebox metadata support"
diff --git a/common/Makefile b/common/Makefile
index 0e0ba384c9b5..8d5a3a20b79a 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_MACHINE_ID) += machine_id.o
obj-$(CONFIG_AUTO_COMPLETE) += complete.o
obj-y += version.o
obj-$(CONFIG_BAREBOX_UPDATE) += bbu.o
+obj-$(CONFIG_MACHINE_PARTITIONS) += machine-partitions.o
obj-$(CONFIG_BINFMT) += binfmt.o
obj-$(CONFIG_BLOCK) += block.o
obj-$(CONFIG_BLSPEC) += blspec.o
diff --git a/common/file-list.c b/common/file-list.c
index 924903cef7dc..580423aef72d 100644
--- a/common/file-list.c
+++ b/common/file-list.c
@@ -170,6 +170,24 @@ void file_list_free(struct file_list *files)
free(files);
}
+struct file_list *file_list_dup(struct file_list *old)
+{
+ struct file_list_entry *old_entry;
+ struct file_list *new;
+
+ new = xzalloc(sizeof(*new));
+
+ INIT_LIST_HEAD(&new->list);
+
+ list_for_each_entry(old_entry, &old->list, list) {
+ (void)file_list_add_entry(new, old_entry->name, old_entry->filename,
+ old_entry->flags); /* can't fail */
+ new->num_entries++;
+ }
+
+ return new;
+}
+
char *file_list_to_str(const struct file_list *files)
{
struct file_list_entry *entry;
diff --git a/common/machine-partitions.c b/common/machine-partitions.c
new file mode 100644
index 000000000000..4179dcd1b045
--- /dev/null
+++ b/common/machine-partitions.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
+ */
+
+#include <file-list.h>
+#include <param.h>
+#include <of.h>
+#include <init.h>
+#include <magicvar.h>
+#include <machine-partitions.h>
+
+static struct file_list *machine_partitions;
+
+bool machine_partitions_empty(void)
+{
+ return file_list_empty(machine_partitions);
+}
+
+struct file_list *machine_partitions_get(void)
+{
+ return file_list_dup(machine_partitions);
+}
+
+void machine_partitions_set(struct file_list *files)
+{
+ file_list_free(machine_partitions);
+ machine_partitions = files;
+}
+
+static int machine_partitions_var_init(void)
+{
+ struct param_d *param;
+
+ machine_partitions = file_list_parse("");
+ param = dev_add_param_file_list(of_get_machine(), "partitions",
+ NULL, NULL, &machine_partitions, NULL);
+
+ return PTR_ERR_OR_ZERO(param);
+}
+postcore_initcall(machine_partitions_var_init);
+
+BAREBOX_MAGICVAR(machine.partitions,
+ "board-specific list of updatable partitions");
diff --git a/include/file-list.h b/include/file-list.h
index 7264a3e2c628..2538883c3659 100644
--- a/include/file-list.h
+++ b/include/file-list.h
@@ -28,6 +28,8 @@ void file_list_free(struct file_list *);
int file_list_add_entry(struct file_list *files, const char *name, const char *filename,
unsigned long flags);
+struct file_list *file_list_dup(struct file_list *old);
+
struct file_list_entry *file_list_entry_by_name(struct file_list *files, const char *name);
#define file_list_for_each_entry(files, entry) \
diff --git a/include/machine-partitions.h b/include/machine-partitions.h
new file mode 100644
index 000000000000..0619a7985bcc
--- /dev/null
+++ b/include/machine-partitions.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef MACHINE_PARTITIONS_H_
+#define MACHINE_PARTITIONS_H_
+
+#include <file-list.h>
+
+#ifdef CONFIG_MACHINE_PARTITIONS
+
+/* duplicates current machine_partitions and returns it */
+struct file_list *machine_partitions_get(void);
+
+/* takes ownership of files and store it internally */
+void machine_partitions_set(struct file_list *files);
+
+/*
+ * check whether machine_partitions_get would return an empty
+ * file_list without doing an allocation
+ */
+bool machine_partitions_empty(void);
+
+#else
+
+static inline struct file_list *machine_partitions_get(void)
+{
+ return file_list_parse("");
+}
+
+static inline bool machine_partitions_empty(void)
+{
+ return true;
+}
+
+/*
+ * machine_partitions_set() intentionally left unimplemented.
+ * select CONFIG_MACHINE_PARTITIONS if you want to set it
+ */
+
+#endif
+
+#endif
diff --git a/include/of.h b/include/of.h
index 645f429bdeed..6bb3d444e4a5 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1066,4 +1066,9 @@ static inline int of_firmware_load_overlay(struct device_node *overlay, const ch
}
#endif
+static inline struct device_d *of_get_machine(void)
+{
+ return of_find_device_by_node(of_get_root_node());
+}
+
#endif /* __OF_H */
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 07/14] fastboot: handle ill-named partitions gracefully
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (5 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 06/14] common: add generic machine partitions interface Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 08/14] usb: gadget: dfu: change status message to info log level Ahmad Fatoum
` (6 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Users can configure a partition name of bbu-something that would
conflict at fastboot init time with a barebox update "something"
handler. Current behavior is to silently ignore all remaining
barebox update handlers.
It would be better to complain loudly and to skip only the entries
actually conflicting. Do so.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/fastboot.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/fastboot.c b/common/fastboot.c
index a394d07e28df..c8576a8d97c3 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -158,10 +158,12 @@ static int fastboot_add_bbu_variables(struct bbu_handler *handler, void *ctx)
name = basprintf("bbu-%s", handler->name);
ret = file_list_add_entry(fb->files, name, handler->devicefile, 0);
+ if (ret)
+ pr_warn("duplicate partition name %s\n", name);
free(name);
- return ret;
+ return 0;
}
int fastboot_generic_init(struct fastboot *fb, bool export_bbu)
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 08/14] usb: gadget: dfu: change status message to info log level
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (6 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 07/14] fastboot: handle ill-named partitions gracefully Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 09/14] usbgadget: autostart: fix indeterminism around usbgadget.autostart Ahmad Fatoum
` (5 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
This used to be a printf, but was changed to pr_err in
f6f521ec38ea ("usb: gadget: dfu: Rework print messages").
This is likely unintended as this is an expected output.
Change it to pr_info.
Fixes: f6f521ec38ea ("usb: gadget: dfu: Rework print messages")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/usb/gadget/dfu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index bb0b34aa9440..7388e6f1525b 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -446,8 +446,7 @@ dfu_bind(struct usb_configuration *c, struct usb_function *f)
i = 0;
file_list_for_each_entry(dfu_files, fentry) {
- pr_err("register alt%d(%s) with device %s\n",
- i, fentry->name, fentry->filename);
+ pr_info("register alt%d(%s) with device %s\n", i, fentry->name, fentry->filename);
i++;
}
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 09/14] usbgadget: autostart: fix indeterminism around usbgadget.autostart
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (7 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 08/14] usb: gadget: dfu: change status message to info log level Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 10/14] usbgadget: allow DFU and Fastboot functions to coexist Ahmad Fatoum
` (4 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
The setter for usbgadget.autostart evaluates other device paramaters, so
it must happen postenvironment, otherwise it could run too early and not
see the device parameters it depends on. This was observed with
usbgadget.dfu_function, which wasn't loaded from the environment early
enough, but it now does.
While at it, remove some more wonkyness:
- usbgadget_autostart_set is only ever called when the
IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART), so drop the check
- There is no need to make usbgadget.acm specific to autostart.
It's behavior now is counter intuitive (enable Kconfig symbol,
but don't set global variable and it will have an effect.
Disable CONFIG_USB_GADGET_AUTOSTART, which looks completely
unrelated and it no longer works.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/usbgadget.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/common/usbgadget.c b/common/usbgadget.c
index feec0b6634be..0b2d9a2120f7 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -97,7 +97,7 @@ static int usbgadget_autostart_set(struct param_d *param, void *ctx)
bool fastboot_bbu = get_fastboot_bbu();
int err;
- if (!IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART) || !autostart || started)
+ if (!autostart || started)
return 0;
err = usbgadget_register(true, NULL, true, NULL, acm, fastboot_bbu);
@@ -109,17 +109,21 @@ static int usbgadget_autostart_set(struct param_d *param, void *ctx)
static int usbgadget_globalvars_init(void)
{
- if (IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART)) {
- globalvar_add_bool("usbgadget.autostart", usbgadget_autostart_set,
- &autostart, NULL);
- globalvar_add_simple_bool("usbgadget.acm", &acm);
- }
+ globalvar_add_simple_bool("usbgadget.acm", &acm);
globalvar_add_simple_string("usbgadget.dfu_function", &dfu_function);
return 0;
}
device_initcall(usbgadget_globalvars_init);
+static int usbgadget_autostart_init(void)
+{
+ if (IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART))
+ globalvar_add_bool("usbgadget.autostart", usbgadget_autostart_set, &autostart, NULL);
+ return 0;
+}
+postenvironment_initcall(usbgadget_autostart_init);
+
BAREBOX_MAGICVAR(global.usbgadget.autostart,
"usbgadget: Automatically start usbgadget on boot");
BAREBOX_MAGICVAR(global.usbgadget.acm,
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 10/14] usbgadget: allow DFU and Fastboot functions to coexist
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (8 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 09/14] usbgadget: autostart: fix indeterminism around usbgadget.autostart Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 11/14] fastboot/dfu: use machine partitions as fall back Ahmad Fatoum
` (3 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
According to the commit, one upon a time the fastboot client tool
did not support a device that exports DFU as well. It does nowadays,
and while dfu-util 0.9 doesn't, it might some day.
Because these host tools are outside of barebox' control, allow both to
coexist and just throw a warning that dfu-util might not work.
With the new machine partitions support, enabled fastboot and DFU mean
that autostart would start both as part of the same multi-gadget. This
would've failed, but would now just emit a warning.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/usbgadget.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/common/usbgadget.c b/common/usbgadget.c
index 0b2d9a2120f7..009debd93efc 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -52,14 +52,13 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
return COMMAND_ERROR_USAGE;
/*
- * Creating a gadget with both DFU and Fastboot doesn't work.
- * Both client tools seem to assume that the device only has
- * a single configuration
+ * Creating a gadget with both DFU and Fastboot may not work.
+ * fastboot 1:8.1.0+r23-5 can deal with it, but dfu-util 0.9
+ * seems to assume that the device only has a single configuration
+ * That's not our fault though. Emit a warning and continue
*/
- if (fastboot_opts && dfu_opts) {
- pr_err("Only one of Fastboot and DFU allowed\n");
- return -EINVAL;
- }
+ if (fastboot_opts && dfu_opts)
+ pr_warn("Both DFU and Fastboot enabled. dfu-util may not like this!\n");
opts = xzalloc(sizeof(*opts));
opts->release = usb_multi_opts_release;
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 11/14] fastboot/dfu: use machine partitions as fall back
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (9 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 10/14] usbgadget: allow DFU and Fastboot functions to coexist Ahmad Fatoum
@ 2021-04-12 22:34 ` Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 12/14] bbu: add function to directly add handlers into file_list Ahmad Fatoum
` (2 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:34 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Use the new machine partitions infrastructure to have fastboot and DFU
fall back to using the same partitions if the global.usbgadget.dfu_function
and global.fastboot_partitions are not set, respectively.
No functional change intended for configurations that have
MACHINE_PARTITIONS disabled.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/fastboot.c | 9 ++++--
common/usbgadget.c | 64 +++++++++++++++++++++-----------------
drivers/usb/gadget/multi.c | 12 +++++++
include/fastboot.h | 6 ++--
include/file-list.h | 5 +++
include/usb/gadget-multi.h | 1 +
net/fastboot.c | 4 +--
7 files changed, 64 insertions(+), 37 deletions(-)
diff --git a/common/fastboot.c b/common/fastboot.c
index c8576a8d97c3..16eda93b93e4 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -41,6 +41,7 @@
#include <linux/stat.h>
#include <linux/mtd/mtd.h>
#include <fastboot.h>
+#include <machine-partitions.h>
#define FASTBOOT_VERSION "0.4"
@@ -932,9 +933,13 @@ bool get_fastboot_bbu(void)
return fastboot_bbu;
}
-const char *get_fastboot_partitions(void)
+struct file_list *get_fastboot_partitions(void)
{
- return fastboot_partitions;
+ if (fastboot_partitions && *fastboot_partitions)
+ return file_list_parse(fastboot_partitions);
+ if (!machine_partitions_empty())
+ return machine_partitions_get();
+ return NULL;
}
static int fastboot_globalvars_init(void)
diff --git a/common/usbgadget.c b/common/usbgadget.c
index 009debd93efc..7aa9a7b9ef1e 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -17,6 +17,7 @@
#include <usb/gadget-multi.h>
#include <globalvar.h>
#include <magicvar.h>
+#include <machine-partitions.h>
static int autostart;
static int acm;
@@ -32,6 +33,15 @@ static struct file_list *parse(const char *files)
return list;
}
+static inline struct file_list *get_dfu_function(void)
+{
+ if (dfu_function && *dfu_function)
+ return file_list_parse(dfu_function);
+ if (!machine_partitions_empty())
+ return machine_partitions_get();
+ return NULL;
+}
+
int usbgadget_register(bool dfu, const char *dfu_opts,
bool fastboot, const char *fastboot_opts,
bool acm, bool export_bbu)
@@ -39,45 +49,37 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
int ret;
struct device_d *dev;
struct f_multi_opts *opts;
- const char *fastboot_partitions = get_fastboot_partitions();
-
- if (dfu && !dfu_opts && dfu_function && *dfu_function)
- dfu_opts = dfu_function;
-
- if (IS_ENABLED(CONFIG_FASTBOOT_BASE) && fastboot && !fastboot_opts &&
- fastboot_partitions && *fastboot_partitions)
- fastboot_opts = fastboot_partitions;
-
- if (!dfu_opts && !fastboot_opts && !acm)
- return COMMAND_ERROR_USAGE;
-
- /*
- * Creating a gadget with both DFU and Fastboot may not work.
- * fastboot 1:8.1.0+r23-5 can deal with it, but dfu-util 0.9
- * seems to assume that the device only has a single configuration
- * That's not our fault though. Emit a warning and continue
- */
- if (fastboot_opts && dfu_opts)
- pr_warn("Both DFU and Fastboot enabled. dfu-util may not like this!\n");
opts = xzalloc(sizeof(*opts));
opts->release = usb_multi_opts_release;
- if (fastboot_opts) {
- opts->fastboot_opts.files = parse(fastboot_opts);
+ if (dfu)
+ opts->dfu_opts.files = dfu_opts ? parse(dfu_opts)
+ : get_dfu_function();
+
+ if (IS_ENABLED(CONFIG_FASTBOOT_BASE) && fastboot) {
+ opts->fastboot_opts.files = fastboot_opts ? parse(fastboot_opts)
+ : get_fastboot_partitions();
+
opts->fastboot_opts.export_bbu = export_bbu;
}
- if (dfu_opts)
- opts->dfu_opts.files = parse(dfu_opts);
+ opts->create_acm = acm;
- if (!opts->dfu_opts.files && !opts->fastboot_opts.files && !acm) {
+ if (usb_multi_count_functions(opts) == 0) {
pr_warn("No functions to register\n");
- free(opts);
- return 0;
+ ret = COMMAND_ERROR_USAGE;
+ goto err;
}
- opts->create_acm = acm;
+ /*
+ * Creating a gadget with both DFU and Fastboot may not work.
+ * fastboot 1:8.1.0+r23-5 can deal with it, but dfu-util 0.9
+ * seems to assume that the device only has a single configuration
+ * That's not our fault though. Emit a warning and continue
+ */
+ if (!file_list_empty(opts->fastboot_opts.files) && !file_list_empty(opts->dfu_opts.files))
+ pr_warn("Both DFU and Fastboot enabled. dfu-util may not like this!\n");
dev = get_device_by_name("otg");
if (dev)
@@ -85,7 +87,11 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
ret = usb_multi_register(opts);
if (ret)
- usb_multi_opts_release(opts);
+ goto err;
+
+ return 0;
+err:
+ usb_multi_opts_release(opts);
return ret;
}
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 95f5b90c88b5..3fb22486a0e9 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -266,6 +266,18 @@ void usb_multi_unregister(void)
gadget_multi_opts = NULL;
}
+unsigned usb_multi_count_functions(struct f_multi_opts *opts)
+{
+ unsigned count = 0;
+
+ count += !file_list_empty(opts->fastboot_opts.files) ||
+ opts->fastboot_opts.export_bbu;
+ count += !file_list_empty(opts->dfu_opts.files);
+ count += opts->create_acm;
+
+ return count;
+}
+
void usb_multi_opts_release(struct f_multi_opts *opts)
{
if (opts->fastboot_opts.files)
diff --git a/include/fastboot.h b/include/fastboot.h
index 2eab2dfe6ae8..cf8a177bf12c 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -58,16 +58,16 @@ enum fastboot_msg_type {
#ifdef CONFIG_FASTBOOT_BASE
bool get_fastboot_bbu(void);
-const char *get_fastboot_partitions(void);
+struct file_list *get_fastboot_partitions(void);
#else
static inline int get_fastboot_bbu(void)
{
return false;
}
-static inline const char *get_fastboot_partitions(void)
+static inline struct file_list *get_fastboot_partitions(void)
{
- return NULL;
+ return file_list_parse("");
}
#endif
diff --git a/include/file-list.h b/include/file-list.h
index 2538883c3659..be97a49b7a2b 100644
--- a/include/file-list.h
+++ b/include/file-list.h
@@ -35,4 +35,9 @@ struct file_list_entry *file_list_entry_by_name(struct file_list *files, const c
#define file_list_for_each_entry(files, entry) \
list_for_each_entry(entry, &files->list, list)
+static inline bool file_list_empty(struct file_list *files)
+{
+ return !files || !files->num_entries;
+}
+
#endif /* __FILE_LIST */
diff --git a/include/usb/gadget-multi.h b/include/usb/gadget-multi.h
index 9bb6c889f3e9..f30dae5686ae 100644
--- a/include/usb/gadget-multi.h
+++ b/include/usb/gadget-multi.h
@@ -15,6 +15,7 @@ struct f_multi_opts {
int usb_multi_register(struct f_multi_opts *opts);
void usb_multi_unregister(void);
void usb_multi_opts_release(struct f_multi_opts *opts);
+unsigned usb_multi_count_functions(struct f_multi_opts *opts);
int usbgadget_register(bool dfu, const char *dfu_opts,
bool fastboot, const char *fastboot_opts,
diff --git a/net/fastboot.c b/net/fastboot.c
index 9082aa48f6e1..df388adc8995 100644
--- a/net/fastboot.c
+++ b/net/fastboot.c
@@ -499,7 +499,6 @@ void fastboot_net_free(struct fastboot_net *fbn)
struct fastboot_net *fastboot_net_init(struct fastboot_opts *opts)
{
struct fastboot_net *fbn;
- const char *partitions = get_fastboot_partitions();
bool bbu = get_fastboot_bbu();
int ret;
@@ -513,8 +512,7 @@ struct fastboot_net *fastboot_net_init(struct fastboot_opts *opts)
fbn->fastboot.cmd_flash = opts->cmd_flash;
ret = fastboot_generic_init(&fbn->fastboot, opts->export_bbu);
} else {
- fbn->fastboot.files = file_list_parse(partitions ?
- partitions : "");
+ fbn->fastboot.files = get_fastboot_partitions() ?: file_list_parse("");
ret = fastboot_generic_init(&fbn->fastboot, bbu);
}
if (ret)
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 12/14] bbu: add function to directly add handlers into file_list
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (10 preceding siblings ...)
2021-04-12 22:34 ` [PATCH 11/14] fastboot/dfu: use machine partitions as fall back Ahmad Fatoum
@ 2021-04-12 22:35 ` Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 13/14] file_list: add file_list_detect_all() Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 14/14] common: make FILE_LIST feature unconditional Ahmad Fatoum
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:35 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
bbu_handlers_iterate() is only used for merging handlers into a
file_list. This can be useful for other update mechanisms as well.
Export a bbu_append_handlers_to_file_list that does this.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/bbu.c | 24 +++++++++++++++---------
common/fastboot.c | 21 ++-------------------
include/bbu.h | 10 +++++++++-
3 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/common/bbu.c b/common/bbu.c
index ee9f78ecc91c..1a1edda96b6c 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -16,22 +16,28 @@
#include <malloc.h>
#include <linux/stat.h>
#include <image-metadata.h>
+#include <file-list.h>
static LIST_HEAD(bbu_image_handlers);
-int bbu_handlers_iterate(int (*fn)(struct bbu_handler *, void *), void *ctx)
+static void append_bbu_entry(struct bbu_handler *handler, struct file_list *files)
{
- struct bbu_handler *handler;
+ char *name;
- list_for_each_entry(handler, &bbu_image_handlers, list) {
- int ret;
+ name = basprintf("bbu-%s", handler->name);
- ret = fn(handler, ctx);
- if (ret)
- return ret;
- }
+ if (file_list_add_entry(files, name, handler->devicefile, 0))
+ pr_warn("duplicate partition name %s\n", name);
- return 0;
+ free(name);
+}
+
+void bbu_append_handlers_to_file_list(struct file_list *files)
+{
+ struct bbu_handler *handler;
+
+ list_for_each_entry(handler, &bbu_image_handlers, list)
+ append_bbu_entry(handler, files);
}
int bbu_force(struct bbu_data *data, const char *fmt, ...)
diff --git a/common/fastboot.c b/common/fastboot.c
index 16eda93b93e4..b002a7fa713c 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -150,23 +150,6 @@ out:
return ret;
}
-static int fastboot_add_bbu_variables(struct bbu_handler *handler, void *ctx)
-{
- struct fastboot *fb = ctx;
- char *name;
- int ret;
-
- name = basprintf("bbu-%s", handler->name);
-
- ret = file_list_add_entry(fb->files, name, handler->devicefile, 0);
- if (ret)
- pr_warn("duplicate partition name %s\n", name);
-
- free(name);
-
- return 0;
-}
-
int fastboot_generic_init(struct fastboot *fb, bool export_bbu)
{
int ret;
@@ -188,8 +171,8 @@ int fastboot_generic_init(struct fastboot *fb, bool export_bbu)
if (!fb->tempname)
return -ENOMEM;
- if (IS_ENABLED(CONFIG_BAREBOX_UPDATE) && export_bbu)
- bbu_handlers_iterate(fastboot_add_bbu_variables, fb);
+ if (export_bbu)
+ bbu_append_handlers_to_file_list(fb->files);
file_list_for_each_entry(fb->files, fentry) {
ret = fastboot_add_partition_variables(fb, fentry);
diff --git a/include/bbu.h b/include/bbu.h
index 3b9d2f4bf151..3128339068ee 100644
--- a/include/bbu.h
+++ b/include/bbu.h
@@ -48,7 +48,7 @@ struct bbu_handler *bbu_find_handler_by_device(const char *devicepath);
void bbu_handlers_list(void);
-int bbu_handlers_iterate(int (*fn)(struct bbu_handler *, void *), void *);
+struct file_list;
#ifdef CONFIG_BAREBOX_UPDATE
@@ -57,6 +57,8 @@ int bbu_register_handler(struct bbu_handler *);
int bbu_register_std_file_update(const char *name, unsigned long flags,
const char *devicefile, enum filetype imagetype);
+void bbu_append_handlers_to_file_list(struct file_list *files);
+
#else
static inline int bbu_register_handler(struct bbu_handler *unused)
@@ -69,6 +71,12 @@ static inline int bbu_register_std_file_update(const char *name, unsigned long f
{
return -ENOSYS;
}
+
+static inline void bbu_append_handlers_to_file_list(struct file_list *files)
+{
+ /* none could be registered, so nothing to do */
+}
+
#endif
#if defined(CONFIG_BAREBOX_UPDATE_IMX_NAND_FCB)
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 13/14] file_list: add file_list_detect_all()
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (11 preceding siblings ...)
2021-04-12 22:35 ` [PATCH 12/14] bbu: add function to directly add handlers into file_list Ahmad Fatoum
@ 2021-04-12 22:35 ` Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 14/14] common: make FILE_LIST feature unconditional Ahmad Fatoum
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:35 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
This is a common theme along the code that uses file_lists.
Add a function that abstracts it. This could later be used
to simplify this operation in the fastboot code.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
common/file-list.c | 17 +++++++++++++++++
include/file-list.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/common/file-list.c b/common/file-list.c
index 580423aef72d..bf1db94e798a 100644
--- a/common/file-list.c
+++ b/common/file-list.c
@@ -8,6 +8,7 @@
#include <file-list.h>
#include <stringlist.h>
#include <linux/err.h>
+#include <driver.h>
#define PARSE_DEVICE 0
#define PARSE_NAME 1
@@ -214,3 +215,19 @@ out:
return str;
}
+
+int file_list_detect_all(const struct file_list *files)
+{
+ struct file_list_entry *fentry;
+ struct stat s;
+ int i = 0;
+
+ list_for_each_entry(fentry, &files->list, list) {
+ if (stat(fentry->filename, &s)) {
+ if (device_detect_by_name(devpath_to_name(fentry->filename)) == 0)
+ i++;
+ }
+ }
+
+ return i;
+}
diff --git a/include/file-list.h b/include/file-list.h
index be97a49b7a2b..7e2a4d9205d7 100644
--- a/include/file-list.h
+++ b/include/file-list.h
@@ -30,6 +30,8 @@ int file_list_add_entry(struct file_list *files, const char *name, const char *f
struct file_list *file_list_dup(struct file_list *old);
+int file_list_detect_all(const struct file_list *files);
+
struct file_list_entry *file_list_entry_by_name(struct file_list *files, const char *name);
#define file_list_for_each_entry(files, entry) \
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 14/14] common: make FILE_LIST feature unconditional
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
` (12 preceding siblings ...)
2021-04-12 22:35 ` [PATCH 13/14] file_list: add file_list_detect_all() Ahmad Fatoum
@ 2021-04-12 22:35 ` Ahmad Fatoum
13 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-12 22:35 UTC (permalink / raw)
To: barebox
From: Ahmad Fatoum <ahmad@a3f.at>
CONFIG_FILE_LIST controls whether the file_list_* family of functions
are compiled. common/file-list.o does not register any initcalls and
there is no code that is dependent on it being available: it's selected
as required. This means linker GC can completely get rid of it if
required, so drop the symbol.
Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
common/Kconfig | 4 ----
common/Makefile | 2 +-
drivers/usb/gadget/Kconfig | 2 --
net/Kconfig | 1 -
4 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig
index f9803dbb5219..8e14649ee27b 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -80,9 +80,6 @@ config EFI_GUID
config EFI_DEVICEPATH
bool
-config FILE_LIST
- bool
-
config ARCH_DMA_ADDR_T_64BIT
bool
@@ -106,7 +103,6 @@ config USBGADGET_START
bool
depends on CMD_USBGADGET || USB_GADGET_AUTOSTART
select ENVIRONMENT_VARIABLES
- select FILE_LIST
default y
config BOOT
diff --git a/common/Makefile b/common/Makefile
index 8d5a3a20b79a..181c27a59e98 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -65,7 +65,7 @@ obj-$(CONFIG_EFI_GUID) += efi-guid.o
obj-$(CONFIG_EFI_DEVICEPATH) += efi-devicepath.o
lwl-$(CONFIG_IMD) += imd-barebox.o
obj-$(CONFIG_IMD) += imd.o
-obj-$(CONFIG_FILE_LIST) += file-list.o
+obj-y += file-list.o
obj-$(CONFIG_FIRMWARE) += firmware.o
obj-$(CONFIG_UBIFORMAT) += ubiformat.o
obj-$(CONFIG_BAREBOX_UPDATE_IMX_NAND_FCB) += imx-bbu-nand-fcb.o
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 7e0c570914d7..4ed6cbbee1b9 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -45,7 +45,6 @@ comment "USB Gadget drivers"
config USB_GADGET_DFU
bool
- select FILE_LIST
prompt "Device Firmware Update Gadget"
config USB_GADGET_SERIAL
@@ -56,7 +55,6 @@ config USB_GADGET_SERIAL
config USB_GADGET_FASTBOOT
bool
select BANNER
- select FILE_LIST
select FASTBOOT_BASE
prompt "Android Fastboot USB Gadget"
endif
diff --git a/net/Kconfig b/net/Kconfig
index 1549c9af6bc1..3512055c456a 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -35,7 +35,6 @@ config NET_SNTP
config NET_FASTBOOT
bool
select BANNER
- select FILE_LIST
select FASTBOOT_BASE
prompt "Android Fastboot support"
help
--
2.29.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 06/14] common: add generic machine partitions interface
2021-04-12 22:34 ` [PATCH 06/14] common: add generic machine partitions interface Ahmad Fatoum
@ 2021-04-14 10:08 ` Sascha Hauer
2021-04-14 10:20 ` Ahmad Fatoum
0 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2021-04-14 10:08 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
On Tue, Apr 13, 2021 at 12:34:54AM +0200, Ahmad Fatoum wrote:
> Both Fastboot and DFU have their own global variables that allow
> specifying the partitions that can be flashed via the environment.
> With the upcoming addition of the USB mass storage gadget, we will need
> some way to define the partitions there as well.
>
> Instead of adding yet another way download method-specific variable,
> add a generic machine.partitions variable that can be specified on a
> per-board basis and can be used for all methods.
>
> Existing variables will still remain for backwards-compatibility, but
> when unset, it should fall back to this new parameter. This is done
> if the follow-up patches.
>
> +static int machine_partitions_var_init(void)
> +{
> + struct param_d *param;
> +
> + machine_partitions = file_list_parse("");
> + param = dev_add_param_file_list(of_get_machine(), "partitions",
> + NULL, NULL, &machine_partitions, NULL);
Why machine.partitions and not global.something? What's the very good
reason to open up another namespace for configuration variables?
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] 21+ messages in thread
* Re: [PATCH 06/14] common: add generic machine partitions interface
2021-04-14 10:08 ` Sascha Hauer
@ 2021-04-14 10:20 ` Ahmad Fatoum
0 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-14 10:20 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hi,
On 14.04.21 12:08, Sascha Hauer wrote:
> On Tue, Apr 13, 2021 at 12:34:54AM +0200, Ahmad Fatoum wrote:
>> Both Fastboot and DFU have their own global variables that allow
>> specifying the partitions that can be flashed via the environment.
>> With the upcoming addition of the USB mass storage gadget, we will need
>> some way to define the partitions there as well.
>>
>> Instead of adding yet another way download method-specific variable,
>> add a generic machine.partitions variable that can be specified on a
>> per-board basis and can be used for all methods.
>>
>> Existing variables will still remain for backwards-compatibility, but
>> when unset, it should fall back to this new parameter. This is done
>> if the follow-up patches.
>>
>> +static int machine_partitions_var_init(void)
>> +{
>> + struct param_d *param;
>> +
>> + machine_partitions = file_list_parse("");
>> + param = dev_add_param_file_list(of_get_machine(), "partitions",
>> + NULL, NULL, &machine_partitions, NULL);
>
> Why machine.partitions and not global.something? What's the very good
> reason to open up another namespace for configuration variables?
I'll go with global.system.partitions. That way it's clear it's machine-specific
(system) and it's still within global.*.
Looking forward to any other feedback before I send v2.
Cheers,
Ahmad
>
> 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] 21+ messages in thread
* Re: [PATCH 02/14] common: console: add log_dprint to print to file descriptor
2021-04-12 22:34 ` [PATCH 02/14] common: console: add log_dprint to print to file descriptor Ahmad Fatoum
@ 2021-04-15 7:25 ` Sascha Hauer
2021-04-15 7:43 ` Ahmad Fatoum
0 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2021-04-15 7:25 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
On Tue, Apr 13, 2021 at 12:34:50AM +0200, Ahmad Fatoum wrote:
> It can be useful to dump the log into the file, e.g. when doing an
> update from a USB flash drive with no serial peer attached. Board code
> could use log_dprint to dump the log onto the drive. Add a function
> to facilitate this.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> common/console_common.c | 15 +++++++++++++++
> include/printk.h | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/common/console_common.c b/common/console_common.c
> index 3e0741572398..77ad4728fbfd 100644
> --- a/common/console_common.c
> +++ b/common/console_common.c
> @@ -182,6 +182,21 @@ static int console_common_init(void)
> }
> device_initcall(console_common_init);
>
> +int log_dprint(int fd)
This function isn't used anywhere in this series, I would prefer to
merge it along with a user.
Is printing to an opened filedescriptor the most useful semantics of
this function? This only seems to make sense when other things should be
written to the file as well, otherwise I would assume to pass a path to
this function which then handles open/close also.
Sascha
> +{
> + struct log_entry *log;
> + int nbytes = 0;
> +
> + list_for_each_entry(log, &barebox_logbuf, list) {
> + int ret = dputs(fd, log->msg);
> + if (ret < 0)
> + return ret;
> + nbytes += ret;
> + }
> +
> + return nbytes;
> +}
> +
> void log_print(unsigned flags, unsigned levels)
> {
> struct log_entry *log;
> diff --git a/include/printk.h b/include/printk.h
> index 94a25ec9ebac..798acfdbf188 100644
> --- a/include/printk.h
> +++ b/include/printk.h
> @@ -141,6 +141,7 @@ extern void log_clean(unsigned int limit);
> #define BAREBOX_LOG_PRINT_ALERT BIT(1)
> #define BAREBOX_LOG_PRINT_EMERG BIT(0)
>
> +int log_dprint(int fd);
> void log_print(unsigned flags, unsigned levels);
>
> struct va_format {
> --
> 2.29.2
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
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] 21+ messages in thread
* Re: [PATCH 01/14] show_progress: add system wide progress stage notifier
2021-04-12 22:34 ` [PATCH 01/14] show_progress: add system wide progress stage notifier Ahmad Fatoum
@ 2021-04-15 7:29 ` Sascha Hauer
2021-04-15 7:39 ` Ahmad Fatoum
0 siblings, 1 reply; 21+ messages in thread
From: Sascha Hauer @ 2021-04-15 7:29 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
On Tue, Apr 13, 2021 at 12:34:49AM +0200, Ahmad Fatoum wrote:
> Use case is e.g. board code that wants to register a client to light
> status LEDs to indicate system state when no serial output is available.
> This functionality doesn't increase code size due to linker GC when
> CONFIG_PROGRESS_NOTIFIER is disabled.
>
> There is a generic progress notifier provided that just logs the
> status. This could be shared with the booted kernel via pstore or
> the log as a whole written to a system setup USB drive.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> include/progress.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> lib/Kconfig | 6 ++++++
> lib/show_progress.c | 28 ++++++++++++++++++++++++++++
> 3 files changed, 77 insertions(+)
>
> diff --git a/include/progress.h b/include/progress.h
> index 50b15fb12b4c..7230bd3a9bd5 100644
> --- a/include/progress.h
> +++ b/include/progress.h
> @@ -3,6 +3,8 @@
> #define __PROGRSS_H
>
> #include <linux/types.h>
> +#include <notifier.h>
> +#include <errno.h>
>
> /* Initialize a progress bar. If max > 0 a one line progress
> * bar is printed where 'max' corresponds to 100%. If max == 0
> @@ -15,4 +17,45 @@ void init_progression_bar(loff_t max);
> */
> void show_progress(loff_t now);
>
> +extern struct notifier_head progress_notifier;
> +
> +enum progress_stage {
> + PROGRESS_UNSPECIFIED = 0,
> + PROGRESS_UPDATING,
> + PROGRESS_UPDATE_SUCCESS,
> + PROGRESS_UPDATE_FAIL,
> +};
Neither the commit message nor the function names say it, but here it
seems the progress is only used in context of a software update. Do you
anticipate going other types of progress here as well?
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] 21+ messages in thread
* Re: [PATCH 01/14] show_progress: add system wide progress stage notifier
2021-04-15 7:29 ` Sascha Hauer
@ 2021-04-15 7:39 ` Ahmad Fatoum
0 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-15 7:39 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hi,
On 15.04.21 09:29, Sascha Hauer wrote:
> On Tue, Apr 13, 2021 at 12:34:49AM +0200, Ahmad Fatoum wrote:
>> Use case is e.g. board code that wants to register a client to light
>> status LEDs to indicate system state when no serial output is available.
>> This functionality doesn't increase code size due to linker GC when
>> CONFIG_PROGRESS_NOTIFIER is disabled.
>>
>> There is a generic progress notifier provided that just logs the
>> status. This could be shared with the booted kernel via pstore or
>> the log as a whole written to a system setup USB drive.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> include/progress.h | 43 +++++++++++++++++++++++++++++++++++++++++++
>> lib/Kconfig | 6 ++++++
>> lib/show_progress.c | 28 ++++++++++++++++++++++++++++
>> 3 files changed, 77 insertions(+)
>>
>> diff --git a/include/progress.h b/include/progress.h
>> index 50b15fb12b4c..7230bd3a9bd5 100644
>> --- a/include/progress.h
>> +++ b/include/progress.h
>> @@ -3,6 +3,8 @@
>> #define __PROGRSS_H
>>
>> #include <linux/types.h>
>> +#include <notifier.h>
>> +#include <errno.h>
>>
>> /* Initialize a progress bar. If max > 0 a one line progress
>> * bar is printed where 'max' corresponds to 100%. If max == 0
>> @@ -15,4 +17,45 @@ void init_progression_bar(loff_t max);
>> */
>> void show_progress(loff_t now);
>>
>> +extern struct notifier_head progress_notifier;
>> +
>> +enum progress_stage {
>> + PROGRESS_UNSPECIFIED = 0,
>> + PROGRESS_UPDATING,
>> + PROGRESS_UPDATE_SUCCESS,
>> + PROGRESS_UPDATE_FAIL,
>> +};
>
> Neither the commit message nor the function names say it, but here it
> seems the progress is only used in context of a software update. Do you
> anticipate going other types of progress here as well?
I don't need any other progress stages myself. If the need arises, they
can be added though. Progress notifiees are supposed to ignore stages
they don't know about.
Cheers,
Ahmad
>
> 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] 21+ messages in thread
* Re: [PATCH 02/14] common: console: add log_dprint to print to file descriptor
2021-04-15 7:25 ` Sascha Hauer
@ 2021-04-15 7:43 ` Ahmad Fatoum
0 siblings, 0 replies; 21+ messages in thread
From: Ahmad Fatoum @ 2021-04-15 7:43 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hello Sascha,
On 15.04.21 09:25, Sascha Hauer wrote:
> On Tue, Apr 13, 2021 at 12:34:50AM +0200, Ahmad Fatoum wrote:
>> It can be useful to dump the log into the file, e.g. when doing an
>> update from a USB flash drive with no serial peer attached. Board code
>> could use log_dprint to dump the log onto the drive. Add a function
>> to facilitate this.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> common/console_common.c | 15 +++++++++++++++
>> include/printk.h | 1 +
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/common/console_common.c b/common/console_common.c
>> index 3e0741572398..77ad4728fbfd 100644
>> --- a/common/console_common.c
>> +++ b/common/console_common.c
>> @@ -182,6 +182,21 @@ static int console_common_init(void)
>> }
>> device_initcall(console_common_init);
>>
>> +int log_dprint(int fd)
>
> This function isn't used anywhere in this series, I would prefer to
> merge it along with a user.
The user code won't be upstreamed. It's a vendor-specific update
mechanism. It's still a useful function to have. Recovery via USB
stick seems to be not uncommon (see protonic board code for example)
and boards implementing this would benefit from being able to write
to log and then dump the log onto the USB stick for evaluation.
> Is printing to an opened filedescriptor the most useful semantics of
> this function? This only seems to make sense when other things should be
> written to the file as well, otherwise I would assume to pass a path to
> this function which then handles open/close also.
Hmm, yes. I can make it write to a supplied filename instead.
>
> Sascha
>
>> +{
>> + struct log_entry *log;
>> + int nbytes = 0;
>> +
>> + list_for_each_entry(log, &barebox_logbuf, list) {
>> + int ret = dputs(fd, log->msg);
>> + if (ret < 0)
>> + return ret;
>> + nbytes += ret;
>> + }
>> +
>> + return nbytes;
>> +}
>> +
>> void log_print(unsigned flags, unsigned levels)
>> {
>> struct log_entry *log;
>> diff --git a/include/printk.h b/include/printk.h
>> index 94a25ec9ebac..798acfdbf188 100644
>> --- a/include/printk.h
>> +++ b/include/printk.h
>> @@ -141,6 +141,7 @@ extern void log_clean(unsigned int limit);
>> #define BAREBOX_LOG_PRINT_ALERT BIT(1)
>> #define BAREBOX_LOG_PRINT_EMERG BIT(0)
>>
>> +int log_dprint(int fd);
>> void log_print(unsigned flags, unsigned levels);
>>
>> struct va_format {
>> --
>> 2.29.2
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>>
>
--
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] 21+ messages in thread
end of thread, other threads:[~2021-04-15 7:54 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 22:34 [PATCH 00/14] usb: gadget: refactor to allow easier extension Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 01/14] show_progress: add system wide progress stage notifier Ahmad Fatoum
2021-04-15 7:29 ` Sascha Hauer
2021-04-15 7:39 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 02/14] common: console: add log_dprint to print to file descriptor Ahmad Fatoum
2021-04-15 7:25 ` Sascha Hauer
2021-04-15 7:43 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 03/14] string: implement strstarts along with strends Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 04/14] vsprintf: introduce %m shorthand for "%s", strerror(errno) Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 05/14] param: introduce file-list parameter type Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 06/14] common: add generic machine partitions interface Ahmad Fatoum
2021-04-14 10:08 ` Sascha Hauer
2021-04-14 10:20 ` Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 07/14] fastboot: handle ill-named partitions gracefully Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 08/14] usb: gadget: dfu: change status message to info log level Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 09/14] usbgadget: autostart: fix indeterminism around usbgadget.autostart Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 10/14] usbgadget: allow DFU and Fastboot functions to coexist Ahmad Fatoum
2021-04-12 22:34 ` [PATCH 11/14] fastboot/dfu: use machine partitions as fall back Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 12/14] bbu: add function to directly add handlers into file_list Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 13/14] file_list: add file_list_detect_all() Ahmad Fatoum
2021-04-12 22:35 ` [PATCH 14/14] common: make FILE_LIST feature unconditional Ahmad Fatoum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox