mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/16] USB: gadget: refactor to allow easier extension
@ 2021-05-03 11:48 Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 01/16] show_progress: add system wide progress stage notifier Ahmad Fatoum
                   ` (16 more replies)
  0 siblings, 17 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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 system 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.

 v2 -> v3:
  - squash fixups
  - add new commit to ignore NULL in file_list_free
  - reworked usbgadget_register logic, so usbgadget -D '' with no
    DFU compiled in is not silently ignored
  - fix error checking in file_list_detect_all

 v1 -> v2:
  - log_writefile(filename) instead of log_dprint(fd) (Sascha)
  - rename $machine.partitions to $global.system.partitions (Sascha)
  - add new commit fixing error with appending to files on ext4


Ahmad Fatoum (16):
  show_progress: add system wide progress stage notifier
  common: console: add log_writefile to write log into new file
  string: implement strstarts along with strends
  vsprintf: introduce %m shorthand for "%s", strerror(errno)
  param: introduce file-list parameter type
  common: add generic system 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
  file_list: make freeing a NULL pointer a no-op
  fastboot/dfu: use system 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
  fs: error out when writing on read-only file system

 common/Kconfig              | 15 ++++--
 common/Makefile             |  3 +-
 common/bbu.c                | 24 ++++++----
 common/console_common.c     | 20 ++++++++
 common/fastboot.c           | 28 ++++-------
 common/file-list.c          | 85 +++++++++++++++++++++++++++++++++
 common/system-partitions.c  | 44 ++++++++++++++++++
 common/usbgadget.c          | 93 ++++++++++++++++++++++---------------
 drivers/usb/gadget/Kconfig  |  2 -
 drivers/usb/gadget/dfu.c    |  3 +-
 drivers/usb/gadget/multi.c  | 18 +++++--
 fs/fs.c                     |  6 +--
 include/bbu.h               | 10 +++-
 include/fastboot.h          |  6 +--
 include/file-list.h         | 12 +++++
 include/linux/string.h      | 10 ++++
 include/param.h             | 15 ++++++
 include/printk.h            |  1 +
 include/progress.h          | 43 +++++++++++++++++
 include/string.h            |  1 +
 include/stringlist.h        |  1 +
 include/system-partitions.h | 40 ++++++++++++++++
 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, 571 insertions(+), 89 deletions(-)
 create mode 100644 common/system-partitions.c
 create mode 100644 include/system-partitions.h

-- 
2.29.2


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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/16] show_progress: add system wide progress stage notifier
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 02/16] common: console: add log_writefile to write log into new file Ahmad Fatoum
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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] 20+ messages in thread

* [PATCH 02/16] common: console: add log_writefile to write log into new file
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 01/16] show_progress: add system wide progress stage notifier Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 03/16] string: implement strstarts along with strends Ahmad Fatoum
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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.
Add a function to facilitate this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/console_common.c | 20 ++++++++++++++++++++
 include/printk.h        |  1 +
 2 files changed, 21 insertions(+)

diff --git a/common/console_common.c b/common/console_common.c
index 3e0741572398..b611f4bf9781 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -182,6 +182,26 @@ static int console_common_init(void)
 }
 device_initcall(console_common_init);
 
+int log_writefile(const char *filepath)
+{
+	int ret = 0, nbytes = 0, fd = -1;
+	struct log_entry *log;
+
+	fd = open(filepath, O_WRONLY | O_CREAT | O_TRUNC);
+	if (fd < 0)
+		return -errno;
+
+	list_for_each_entry(log, &barebox_logbuf, list) {
+		ret = dputs(fd, log->msg);
+		if (ret < 0)
+			break;
+		nbytes += ret;
+	}
+
+	close(fd);
+	return ret < 0 ? ret : nbytes;
+}
+
 void log_print(unsigned flags, unsigned levels)
 {
 	struct log_entry *log;
diff --git a/include/printk.h b/include/printk.h
index 94a25ec9ebac..f83ad3bf07f0 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_writefile(const char *filepath);
 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] 20+ messages in thread

* [PATCH 03/16] string: implement strstarts along with strends
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 01/16] show_progress: add system wide progress stage notifier Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 02/16] common: console: add log_writefile to write log into new file Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 04/16] vsprintf: introduce %m shorthand for "%s", strerror(errno) Ahmad Fatoum
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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] 20+ messages in thread

* [PATCH 04/16] vsprintf: introduce %m shorthand for "%s", strerror(errno)
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 03/16] string: implement strstarts along with strends Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 05/16] param: introduce file-list parameter type Ahmad Fatoum
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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] 20+ messages in thread

* [PATCH 05/16] param: introduce file-list parameter type
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 04/16] vsprintf: introduce %m shorthand for "%s", strerror(errno) Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 06/16] common: add generic system partitions interface Ahmad Fatoum
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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.

A dedicated file list parameter would improve the user experience
and makes the code using it easier to handle: the struct file_list
can be passed around directly instead of having to parse it first
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 c5d6e70a367d..01491082ea02 100644
--- a/include/stringlist.h
+++ b/include/stringlist.h
@@ -14,6 +14,7 @@ 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_add_sort_uniq(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 173420c58a56..adc3c7cdea70 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)
@@ -907,6 +910,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 719fecdaa456..fc86640b94a8 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)
@@ -95,6 +96,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] 20+ messages in thread

* [PATCH 06/16] common: add generic system partitions interface
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 05/16] param: introduce file-list parameter type Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 07/16] fastboot: handle ill-named partitions gracefully Ahmad Fatoum
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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 global.system.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
in 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/system-partitions.c  | 44 +++++++++++++++++++++++++++++++++++++
 include/file-list.h         |  2 ++
 include/system-partitions.h | 40 +++++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+)
 create mode 100644 common/system-partitions.c
 create mode 100644 include/system-partitions.h

diff --git a/common/Kconfig b/common/Kconfig
index 342817bbcbb4..1ec221dbfccd 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -707,6 +707,17 @@ config FLEXIBLE_BOOTARGS
 config BAREBOX_UPDATE
 	bool "In-system barebox update infrastructure"
 
+config SYSTEM_PARTITIONS
+	bool "Generic system partitions support"
+	depends on GLOBALVAR
+	help
+	  System partitions are a generic way for boards to specify the
+	  partitions that should be exported for flashing.
+	  Board drivers that set this directly will select this option
+	  automatically.
+	  Say y here if this should be configurable over the
+	  global.system.partitions device parameter as well.
+
 config IMD
 	select CRC32
 	bool "barebox metadata support"
diff --git a/common/Makefile b/common/Makefile
index c0b45d263e5b..dc1c6b9afdd7 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_SYSTEM_PARTITIONS) += system-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/system-partitions.c b/common/system-partitions.c
new file mode 100644
index 000000000000..547e08a9f30b
--- /dev/null
+++ b/common/system-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 <globalvar.h>
+#include <init.h>
+#include <magicvar.h>
+#include <system-partitions.h>
+
+static struct file_list *system_partitions;
+
+bool system_partitions_empty(void)
+{
+	return file_list_empty(system_partitions);
+}
+
+struct file_list *system_partitions_get(void)
+{
+	return file_list_dup(system_partitions);
+}
+
+void system_partitions_set(struct file_list *files)
+{
+	file_list_free(system_partitions);
+	system_partitions = files;
+}
+
+static int system_partitions_var_init(void)
+{
+	struct param_d *param;
+
+	system_partitions = file_list_parse("");
+	param = dev_add_param_file_list(&global_device, "system.partitions",
+					NULL, NULL, &system_partitions, NULL);
+
+	return PTR_ERR_OR_ZERO(param);
+}
+postcore_initcall(system_partitions_var_init);
+
+BAREBOX_MAGICVAR(global.system.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/system-partitions.h b/include/system-partitions.h
new file mode 100644
index 000000000000..86de3612ccd8
--- /dev/null
+++ b/include/system-partitions.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef SYSTEM_PARTITIONS_H_
+#define SYSTEM_PARTITIONS_H_
+
+#include <file-list.h>
+
+#ifdef CONFIG_SYSTEM_PARTITIONS
+
+/* duplicates current system_partitions and returns it */
+struct file_list *system_partitions_get(void);
+
+/* takes ownership of files and store it internally */
+void system_partitions_set(struct file_list *files);
+
+/*
+ * check whether system_partitions_get would return an empty
+ * file_list without doing an allocation
+ */
+bool system_partitions_empty(void);
+
+#else
+
+static inline struct file_list *system_partitions_get(void)
+{
+	return file_list_parse("");
+}
+
+static inline bool system_partitions_empty(void)
+{
+	return true;
+}
+
+/*
+ * system_partitions_set() intentionally left unimplemented.
+ * select CONFIG_SYSTEM_PARTITIONS if you want to set it
+ */
+
+#endif
+
+#endif
-- 
2.29.2


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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 07/16] fastboot: handle ill-named partitions gracefully
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 06/16] common: add generic system partitions interface Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 08/16] usb: gadget: dfu: change status message to info log level Ahmad Fatoum
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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] 20+ messages in thread

* [PATCH 08/16] usb: gadget: dfu: change status message to info log level
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 07/16] fastboot: handle ill-named partitions gracefully Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 09/16] usbgadget: autostart: fix indeterminism around usbgadget.autostart Ahmad Fatoum
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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 1ac63c464206..fd0ec505dc68 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -447,8 +447,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] 20+ messages in thread

* [PATCH 09/16] usbgadget: autostart: fix indeterminism around usbgadget.autostart
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 08/16] usb: gadget: dfu: change status message to info log level Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 10/16] usbgadget: allow DFU and Fastboot functions to coexist Ahmad Fatoum
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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] 20+ messages in thread

* [PATCH 10/16] usbgadget: allow DFU and Fastboot functions to coexist
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 09/16] usbgadget: autostart: fix indeterminism around usbgadget.autostart Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 11/16] file_list: make freeing a NULL pointer a no-op Ahmad Fatoum
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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 system 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] 20+ messages in thread

* [PATCH 11/16] file_list: make freeing a NULL pointer a no-op
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 10/16] usbgadget: allow DFU and Fastboot functions to coexist Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 12/16] fastboot/dfu: use system partitions as fall back Ahmad Fatoum
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

This makes code added into usbgadget in a later commit less verbose.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/file-list.c         | 3 +++
 drivers/usb/gadget/multi.c | 6 ++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/common/file-list.c b/common/file-list.c
index 580423aef72d..55e6f0e6b6c0 100644
--- a/common/file-list.c
+++ b/common/file-list.c
@@ -161,6 +161,9 @@ void file_list_free(struct file_list *files)
 {
 	struct file_list_entry *entry, *tmp;
 
+	if (!files)
+		return;
+
 	list_for_each_entry_safe(entry, tmp, &files->list, list) {
 		free(entry->name);
 		free(entry->filename);
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 95f5b90c88b5..da4a7591175c 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -268,10 +268,8 @@ void usb_multi_unregister(void)
 
 void usb_multi_opts_release(struct f_multi_opts *opts)
 {
-	if (opts->fastboot_opts.files)
-		file_list_free(opts->fastboot_opts.files);
-	if (opts->dfu_opts.files)
-		file_list_free(opts->dfu_opts.files);
+	file_list_free(opts->fastboot_opts.files);
+	file_list_free(opts->dfu_opts.files);
 
 	free(opts);
 }
-- 
2.29.2


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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 12/16] fastboot/dfu: use system partitions as fall back
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (10 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 11/16] file_list: make freeing a NULL pointer a no-op Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 12:30   ` [PATCH] fixup! " Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 13/16] bbu: add function to directly add handlers into file_list Ahmad Fatoum
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Use the new system 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
SYSTEM_PARTITIONS disabled.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c          |  9 ++++-
 common/usbgadget.c         | 76 +++++++++++++++++++++++---------------
 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, 75 insertions(+), 38 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index c8576a8d97c3..dc80b66e6720 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 <system-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 (!system_partitions_empty())
+		return system_partitions_get();
+	return NULL;
 }
 
 static int fastboot_globalvars_init(void)
diff --git a/common/usbgadget.c b/common/usbgadget.c
index 009debd93efc..b953f8bf771c 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 <system-partitions.h>
 
 static int autostart;
 static int acm;
@@ -24,14 +25,29 @@ static char *dfu_function;
 
 static struct file_list *parse(const char *files)
 {
-	struct file_list *list = file_list_parse(files);
+	struct file_list *list;
+
+	if (!files)
+		return NULL;
+
+	list = file_list_parse(files);
 	if (IS_ERR(list)) {
 		pr_err("Parsing file list \"%s\" failed: %pe\n", files, list);
 		return NULL;
 	}
+
 	return list;
 }
 
+static inline struct file_list *get_dfu_function(void)
+{
+	if (dfu_function && *dfu_function)
+		return file_list_parse(dfu_function);
+	if (!system_partitions_empty())
+		return system_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,17 +55,32 @@ 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;
+	opts = xzalloc(sizeof(*opts));
+	opts->release = usb_multi_opts_release;
 
-	if (IS_ENABLED(CONFIG_FASTBOOT_BASE) && fastboot && !fastboot_opts &&
-	    fastboot_partitions && *fastboot_partitions)
-		fastboot_opts = fastboot_partitions;
+	opts->dfu_opts.files = parse(dfu_opts);
+	if (IS_ENABLED(CONFIG_USB_GADGET_DFU) && file_list_empty(opts->dfu_opts.files)) {
+		file_list_free(opts->dfu_opts.files);
+		opts->dfu_opts.files = get_dfu_function();
+	}
+
+	opts->fastboot_opts.files = parse(fastboot_opts);
+	if (IS_ENABLED(CONFIG_FASTBOOT_BASE) && file_list_empty(opts->fastboot_opts.files)) {
+		file_list_free(opts->fastboot_opts.files);
+		opts->fastboot_opts.files = get_fastboot_partitions();
+	}
+
+	if (fastboot)
+		opts->fastboot_opts.export_bbu = export_bbu;
 
-	if (!dfu_opts && !fastboot_opts && !acm)
-		return COMMAND_ERROR_USAGE;
+	opts->create_acm = acm;
+
+	if (usb_multi_count_functions(opts) == 0) {
+		pr_warn("No functions to register\n");
+		ret = COMMAND_ERROR_USAGE;
+		goto err;
+	}
 
 	/*
 	 * Creating a gadget with both DFU and Fastboot may not work.
@@ -57,35 +88,20 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 	 * 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)
+	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");
 
-	opts = xzalloc(sizeof(*opts));
-	opts->release = usb_multi_opts_release;
-
-	if (fastboot_opts) {
-		opts->fastboot_opts.files = parse(fastboot_opts);
-		opts->fastboot_opts.export_bbu = export_bbu;
-	}
-
-	if (dfu_opts)
-		opts->dfu_opts.files = parse(dfu_opts);
-
-	if (!opts->dfu_opts.files && !opts->fastboot_opts.files && !acm) {
-		pr_warn("No functions to register\n");
-		free(opts);
-		return 0;
-	}
-
-	opts->create_acm = acm;
-
 	dev = get_device_by_name("otg");
 	if (dev)
 		dev_set_param(dev, "mode", "peripheral");
 
 	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 da4a7591175c..144ac0624be2 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)
 {
 	file_list_free(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] 20+ messages in thread

* [PATCH 13/16] bbu: add function to directly add handlers into file_list
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (11 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 12/16] fastboot/dfu: use system partitions as fall back Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:48 ` [PATCH 14/16] file_list: add file_list_detect_all() Ahmad Fatoum
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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 dc80b66e6720..75f6691b08b6 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] 20+ messages in thread

* [PATCH 14/16] file_list: add file_list_detect_all()
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (12 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 13/16] bbu: add function to directly add handlers into file_list Ahmad Fatoum
@ 2021-05-03 11:48 ` Ahmad Fatoum
  2021-05-03 11:49 ` [PATCH 15/16] common: make FILE_LIST feature unconditional Ahmad Fatoum
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:48 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 55e6f0e6b6c0..05f44514fb09 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
@@ -217,3 +218,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))
+			continue;
+		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] 20+ messages in thread

* [PATCH 15/16] common: make FILE_LIST feature unconditional
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (13 preceding siblings ...)
  2021-05-03 11:48 ` [PATCH 14/16] file_list: add file_list_detect_all() Ahmad Fatoum
@ 2021-05-03 11:49 ` Ahmad Fatoum
  2021-05-03 11:49 ` [PATCH 16/16] fs: error out when writing on read-only file system Ahmad Fatoum
  2021-05-03 11:55 ` [PATCH 00/16] USB: gadget: refactor to allow easier extension Sascha Hauer
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:49 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

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 <a.fatoum@pengutronix.de>
---
 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 1ec221dbfccd..6a01b3c06b70 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -91,9 +91,6 @@ config EFI_GUID
 config EFI_DEVICEPATH
 	bool
 
-config FILE_LIST
-	bool
-
 config ARCH_DMA_ADDR_T_64BIT
 	bool
 
@@ -117,7 +114,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 dc1c6b9afdd7..c2c15817badf 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -66,7 +66,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] 20+ messages in thread

* [PATCH 16/16] fs: error out when writing on read-only file system
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (14 preceding siblings ...)
  2021-05-03 11:49 ` [PATCH 15/16] common: make FILE_LIST feature unconditional Ahmad Fatoum
@ 2021-05-03 11:49 ` Ahmad Fatoum
  2021-05-03 11:55 ` [PATCH 00/16] USB: gadget: refactor to allow easier extension Sascha Hauer
  16 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 11:49 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

On an ext4 partition, Running

  echo -a /mnt/disk0.0/file test

will crash barebox because fsdrv->truncate in __write is NULL.

Don't let it come to this and verify earlier that the FS implements
->write. While at it, explicitly compare with O_RDONLY (== 0) for
clarity.

Fixes: b3fbfad7aeaf ("fs: dentry cache implementation")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/fs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index a023324913c4..881dc2fca0ce 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -321,13 +321,13 @@ static ssize_t __write(FILE *f, const void *buf, size_t count)
 	struct fs_driver_d *fsdrv;
 	int ret;
 
-	if (!(f->flags & O_ACCMODE)) {
+	fsdrv = f->fsdev->driver;
+
+	if ((f->flags & O_ACCMODE) == O_RDONLY || !fsdrv->write) {
 		ret = -EBADF;
 		goto out;
 	}
 
-	fsdrv = f->fsdev->driver;
-
 	if (fsdrv != ramfs_driver)
 		assert_command_context();
 
-- 
2.29.2


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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/16] USB: gadget: refactor to allow easier extension
  2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
                   ` (15 preceding siblings ...)
  2021-05-03 11:49 ` [PATCH 16/16] fs: error out when writing on read-only file system Ahmad Fatoum
@ 2021-05-03 11:55 ` Sascha Hauer
  16 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2021-05-03 11:55 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, May 03, 2021 at 01:48:45PM +0200, Ahmad Fatoum wrote:
>  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 system 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.
> 
>  v2 -> v3:
>   - squash fixups
>   - add new commit to ignore NULL in file_list_free
>   - reworked usbgadget_register logic, so usbgadget -D '' with no
>     DFU compiled in is not silently ignored
>   - fix error checking in file_list_detect_all
> 
>  v1 -> v2:
>   - log_writefile(filename) instead of log_dprint(fd) (Sascha)
>   - rename $machine.partitions to $global.system.partitions (Sascha)
>   - add new commit fixing error with appending to files on ext4
> 
> 
> Ahmad Fatoum (16):
>   show_progress: add system wide progress stage notifier
>   common: console: add log_writefile to write log into new file
>   string: implement strstarts along with strends
>   vsprintf: introduce %m shorthand for "%s", strerror(errno)
>   param: introduce file-list parameter type
>   common: add generic system 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
>   file_list: make freeing a NULL pointer a no-op
>   fastboot/dfu: use system 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
>   fs: error out when writing on read-only file system

Applied, thanks

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] 20+ messages in thread

* [PATCH] fixup! fastboot/dfu: use system partitions as fall back
  2021-05-03 11:48 ` [PATCH 12/16] fastboot/dfu: use system partitions as fall back Ahmad Fatoum
@ 2021-05-03 12:30   ` Ahmad Fatoum
  0 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-05-03 12:30 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

We need to maintain the check for whether dfu/fastboot is enabled,
otherwise usbgadget_register can't differentiate between disabled
gadget and gadget with default (empty string) function.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/usbgadget.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/common/usbgadget.c b/common/usbgadget.c
index b953f8bf771c..d4437b51694a 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -59,20 +59,23 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 	opts = xzalloc(sizeof(*opts));
 	opts->release = usb_multi_opts_release;
 
-	opts->dfu_opts.files = parse(dfu_opts);
-	if (IS_ENABLED(CONFIG_USB_GADGET_DFU) && file_list_empty(opts->dfu_opts.files)) {
-		file_list_free(opts->dfu_opts.files);
-		opts->dfu_opts.files = get_dfu_function();
+	if (dfu) {
+		opts->dfu_opts.files = parse(dfu_opts);
+		if (IS_ENABLED(CONFIG_USB_GADGET_DFU) && file_list_empty(opts->dfu_opts.files)) {
+			file_list_free(opts->dfu_opts.files);
+			opts->dfu_opts.files = get_dfu_function();
+		}
 	}
 
-	opts->fastboot_opts.files = parse(fastboot_opts);
-	if (IS_ENABLED(CONFIG_FASTBOOT_BASE) && file_list_empty(opts->fastboot_opts.files)) {
-		file_list_free(opts->fastboot_opts.files);
-		opts->fastboot_opts.files = get_fastboot_partitions();
-	}
+	if (fastboot) {
+		opts->fastboot_opts.files = parse(fastboot_opts);
+		if (IS_ENABLED(CONFIG_FASTBOOT_BASE) && file_list_empty(opts->fastboot_opts.files)) {
+			file_list_free(opts->fastboot_opts.files);
+			opts->fastboot_opts.files = get_fastboot_partitions();
+		}
 
-	if (fastboot)
 		opts->fastboot_opts.export_bbu = export_bbu;
+	}
 
 	opts->create_acm = acm;
 
-- 
2.29.2


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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] fixup! fastboot/dfu: use system partitions as fall back
  2021-04-30 13:29 [PATCH v2 11/15] fastboot/dfu: use system partitions as fall back Ahmad Fatoum
@ 2021-04-30 17:57 ` Ahmad Fatoum
  0 siblings, 0 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2021-04-30 17:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

For configuration where DFU is disabled, opts->dfu_opts.files would
contain the system partitions and multi-gadget auto start would
fail. Ignore dfu == true when support is not compiled in.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/usbgadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/usbgadget.c b/common/usbgadget.c
index bdbb4d794889..489b3b801630 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -53,7 +53,7 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 	opts = xzalloc(sizeof(*opts));
 	opts->release = usb_multi_opts_release;
 
-	if (dfu)
+	if (IS_ENABLED(CONFIG_USB_GADGET_DFU) && dfu)
 		opts->dfu_opts.files = dfu_opts ? parse(dfu_opts)
 			: get_dfu_function();
 
-- 
2.29.2


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


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-05-03 12:32 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 11:48 [PATCH 00/16] USB: gadget: refactor to allow easier extension Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 01/16] show_progress: add system wide progress stage notifier Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 02/16] common: console: add log_writefile to write log into new file Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 03/16] string: implement strstarts along with strends Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 04/16] vsprintf: introduce %m shorthand for "%s", strerror(errno) Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 05/16] param: introduce file-list parameter type Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 06/16] common: add generic system partitions interface Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 07/16] fastboot: handle ill-named partitions gracefully Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 08/16] usb: gadget: dfu: change status message to info log level Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 09/16] usbgadget: autostart: fix indeterminism around usbgadget.autostart Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 10/16] usbgadget: allow DFU and Fastboot functions to coexist Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 11/16] file_list: make freeing a NULL pointer a no-op Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 12/16] fastboot/dfu: use system partitions as fall back Ahmad Fatoum
2021-05-03 12:30   ` [PATCH] fixup! " Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 13/16] bbu: add function to directly add handlers into file_list Ahmad Fatoum
2021-05-03 11:48 ` [PATCH 14/16] file_list: add file_list_detect_all() Ahmad Fatoum
2021-05-03 11:49 ` [PATCH 15/16] common: make FILE_LIST feature unconditional Ahmad Fatoum
2021-05-03 11:49 ` [PATCH 16/16] fs: error out when writing on read-only file system Ahmad Fatoum
2021-05-03 11:55 ` [PATCH 00/16] USB: gadget: refactor to allow easier extension Sascha Hauer
  -- strict thread matches above, loose matches on Subject: below --
2021-04-30 13:29 [PATCH v2 11/15] fastboot/dfu: use system partitions as fall back Ahmad Fatoum
2021-04-30 17:57 ` [PATCH] fixup! " Ahmad Fatoum

mail archive of the barebox mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lore.barebox.org/barebox/0 barebox/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 barebox barebox/ https://lore.barebox.org/barebox \
		barebox@lists.infradead.org
	public-inbox-index barebox

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git