[PATCH 00/12] include: common.h: make it include only headers

[PATCH 00/12] include: common.h: make it include only headers

[Ahmad Fatoum]

Symbols exclusively defined in common.h are a problem, because other
headers that require them will need to include a lot of extra baggage,
which in the worst case can lead to cyclic dependencies and in every
case leads to longer compile times.

This series prepare for removing common.h in other headers by moving
everything it contains apart from #includes into more fitting existing
headers that are already being included.

Ahmad Fatoum (12):
  include: common.h: move barebox startup functions into separate header
  include: common.h: move ctrlc() functions into stdio.h
  include: common.h: move out integer string parsing functions
  include: common.h: move out endianness macro sanity check
  include: common.h: move out user interface functions into stdio.h
  include: common.h: move out memory option parsing prototypes
  include: common.h: move out RW_BUF_SIZE definition
  commands: add macro to simplify defining one shot commands
  commands: reginfo: make command mpc5xxx-specific
  include: common.h: move out get_ram_size
  include: align: reword STACK_ALIGN_ARRAY macro parameter for clarity
  ARM: bcm283x: remove common.h include in mbox.h

 arch/arm/boards/raspberry-pi/mbox-helpers.c |  1 +
 arch/powerpc/mach-mpc5xxx/Kconfig           |  4 -
 arch/powerpc/mach-mpc5xxx/Makefile          |  2 +-
 arch/powerpc/mach-mpc5xxx/reginfo.c         |  4 +-
 commands/Kconfig                            | 11 ---
 commands/Makefile                           |  1 -
 commands/reginfo.c                          | 21 ------
 include/barebox.h                           | 37 ++++++++++
 include/command.h                           | 15 ++++
 include/common.h                            | 81 +--------------------
 include/getopt.h                            |  8 ++
 include/linux/align.h                       | 11 +--
 include/linux/kstrtox.h                     |  6 ++
 include/mach/bcm283x/mbox.h                 |  4 +-
 include/stdio.h                             | 14 ++++
 include/unistd.h                            |  2 +
 lib/hexdump.c                               | 20 +++++
 17 files changed, 115 insertions(+), 127 deletions(-)
 delete mode 100644 commands/reginfo.c
 create mode 100644 include/barebox.h

-- 
2.39.5

[PATCH 01/12] include: common.h: move barebox startup functions into separate header

[Ahmad Fatoum]

Symbols exclusively defined in common.h are a problem, because other
headers that require them will need to include a lot of extra baggage,
which in the worst case can lead to cyclic dependencies and in every
case leads to longer compile times.

In preparation for turning common.h into containing only #include lines,
let's move the barebox boot stage stuff into a separate header.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/barebox.h | 35 +++++++++++++++++++++++++++++++++++
 include/common.h  | 28 +---------------------------
 2 files changed, 36 insertions(+), 27 deletions(-)
 create mode 100644 include/barebox.h

diff --git a/include/barebox.h b/include/barebox.h
new file mode 100644
index 000000000000..741463bc1cf9
--- /dev/null
+++ b/include/barebox.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __BAREBOX_H_
+#define __BAREBOX_H_
+
+#include <linux/compiler.h>
+
+/* For use when unrelocated */
+static inline void __hang(void)
+{
+	while (1);
+}
+void __noreturn hang (void);
+
+/*
+ * Function pointer to the main barebox function. Defaults
+ * to run_shell() when a shell is enabled.
+ */
+extern int (*barebox_main)(void);
+
+enum autoboot_state {
+	AUTOBOOT_COUNTDOWN,
+	AUTOBOOT_ABORT,
+	AUTOBOOT_MENU,
+	AUTOBOOT_BOOT,
+	AUTOBOOT_UNKNOWN,
+};
+
+void set_autoboot_state(enum autoboot_state autoboot);
+enum autoboot_state do_autoboot_countdown(void);
+
+void __noreturn start_barebox(void);
+void shutdown_barebox(void);
+
+#endif
diff --git a/include/common.h b/include/common.h
index 571f46fd0bc1..cc7f089aeef0 100644
--- a/include/common.h
+++ b/include/common.h
@@ -8,6 +8,7 @@
 #define __COMMON_H_	1
 
 #include <stdio.h>
+#include <barebox.h>
 #include <module.h>
 #include <config.h>
 #include <clock.h>
@@ -45,14 +46,6 @@
  */
 void reginfo(void);
 
-/* For use when unrelocated */
-static inline void __hang(void)
-{
-	while (1);
-}
-
-void __noreturn hang (void);
-
 char *size_human_readable(unsigned long long size);
 
 int	readline	(const char *prompt, char *buf, int len);
@@ -76,25 +69,6 @@ int parse_area_spec(const char *str, loff_t *start, loff_t *size);
 unsigned long strtoul_suffix(const char *str, char **endp, int base);
 unsigned long long strtoull_suffix(const char *str, char **endp, int base);
 
-/*
- * Function pointer to the main barebox function. Defaults
- * to run_shell() when a shell is enabled.
- */
-extern int (*barebox_main)(void);
-
-enum autoboot_state {
-	AUTOBOOT_COUNTDOWN,
-	AUTOBOOT_ABORT,
-	AUTOBOOT_MENU,
-	AUTOBOOT_BOOT,
-	AUTOBOOT_UNKNOWN,
-};
-
-void set_autoboot_state(enum autoboot_state autoboot);
-enum autoboot_state do_autoboot_countdown(void);
-
-void __noreturn start_barebox(void);
-void shutdown_barebox(void);
 int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
 		char **sourcefile, char **destfile, int *swab);
 int memcpy_parse_options(int argc, char *argv[], int *sourcefd,
-- 
2.39.5

[PATCH 02/12] include: common.h: move ctrlc() functions into stdio.h

[Ahmad Fatoum]

A stub definition for ctrlc() is already in stdio.h and stdio.h is
already included by common.h, so let's move all other ctrlc related
definitions into stdio.h.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h | 10 ----------
 include/stdio.h  | 11 +++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/include/common.h b/include/common.h
index cc7f089aeef0..6baab254c80e 100644
--- a/include/common.h
+++ b/include/common.h
@@ -53,16 +53,6 @@ int	readline	(const char *prompt, char *buf, int len);
 /* common/memsize.c */
 long	get_ram_size  (volatile long *, long);
 
-/* common/console.c */
-int ctrlc(void);
-int arch_ctrlc(void);
-
-#ifdef CONFIG_CONSOLE_FULL
-void ctrlc_handled(void);
-#else
-static inline void ctrlc_handled(void) { }
-#endif
-
 int parse_area_spec(const char *str, loff_t *start, loff_t *size);
 
 /* Just like simple_strtoul(), but this one honors a K/M/G suffix */
diff --git a/include/stdio.h b/include/stdio.h
index 095e9b0a1d42..33ca73a80d18 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -24,6 +24,10 @@ int vasprintf(char **strp, const char *fmt, va_list ap);
 int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
 int vscnprintf(char *buf, size_t size, const char *fmt, va_list args);
 
+#ifdef CONFIG_ARCH_HAS_CTRLC
+int arch_ctrlc(void);
+#endif
+
 #ifndef CONFIG_CONSOLE_NONE
 /* stdin */
 int tstc(void);
@@ -35,6 +39,9 @@ int console_puts(unsigned int ch, const char *s);
 void console_flush(void);
 
 int vprintf(const char *fmt, va_list args);
+
+int ctrlc(void);
+void ctrlc_handled(void);
 #else
 static inline int tstc(void)
 {
@@ -66,6 +73,10 @@ static inline int ctrlc (void)
 	return 0;
 }
 
+static inline void ctrlc_handled(void)
+{
+}
+
 #endif
 
 #if (!defined(__PBL__) && !defined(CONFIG_CONSOLE_NONE)) || \
-- 
2.39.5

[PATCH 03/12] include: common.h: move out integer string parsing functions

[Ahmad Fatoum]

The <linux/kstrtox.h> header is already included into <common.h> via
<linux/kernel.h> and includes a lot more integer parsing functions, so
let's move the few such functions in common.h there as well.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h        | 6 ------
 include/linux/kstrtox.h | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/common.h b/include/common.h
index 6baab254c80e..d8e55114ce23 100644
--- a/include/common.h
+++ b/include/common.h
@@ -53,12 +53,6 @@ int	readline	(const char *prompt, char *buf, int len);
 /* common/memsize.c */
 long	get_ram_size  (volatile long *, long);
 
-int parse_area_spec(const char *str, loff_t *start, loff_t *size);
-
-/* Just like simple_strtoul(), but this one honors a K/M/G suffix */
-unsigned long strtoul_suffix(const char *str, char **endp, int base);
-unsigned long long strtoull_suffix(const char *str, char **endp, int base);
-
 int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
 		char **sourcefile, char **destfile, int *swab);
 int memcpy_parse_options(int argc, char *argv[], int *sourcefd,
diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 51493c71292b..a98f5791a368 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -118,6 +118,12 @@ extern long simple_strtol(const char *,char **,unsigned int);
 extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
 extern long long simple_strtoll(const char *,char **,unsigned int);
 
+/* Just like simple_strtoul(), but this one honors a K/M/G suffix */
+unsigned long strtoul_suffix(const char *str, char **endp, int base);
+unsigned long long strtoull_suffix(const char *str, char **endp, int base);
+
 extern s64 simple_strtofract(const char *cp, char **endp, u32 division);
 
+int parse_area_spec(const char *str, loff_t *start, loff_t *size);
+
 #endif
-- 
2.39.5

[PATCH 04/12] include: common.h: move out endianness macro sanity check

[Ahmad Fatoum]

This sanity check was added into common.h, so it's part of every build.
As we intend to move out everything that's not an include out of
common.h, let's move it somewhere, where it's still included in every
build, but isn't evaluated repeatedly all the time.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h | 18 ------------------
 lib/hexdump.c    | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/include/common.h b/include/common.h
index d8e55114ce23..bab03a0444f7 100644
--- a/include/common.h
+++ b/include/common.h
@@ -23,24 +23,6 @@
 #include <linux/printk.h>
 #include <barebox-info.h>
 
-/*
- * sanity check. The Linux Kernel defines only one of __LITTLE_ENDIAN and
- * __BIG_ENDIAN. Endianess can then be tested with #ifdef __xx_ENDIAN. Userspace
- * always defined both __LITTLE_ENDIAN and __BIG_ENDIAN and byteorder can then
- * be tested with #if __BYTE_ORDER == __xx_ENDIAN.
- *
- * As we tend to use a lot of Kernel code in barebox we use the kernel way of
- * determing the byte order. Make sure here that architecture code properly
- * defines it.
- */
-#include <asm/byteorder.h>
-#if defined __LITTLE_ENDIAN && defined __BIG_ENDIAN
-#error "both __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
-#endif
-#if !defined __LITTLE_ENDIAN && !defined __BIG_ENDIAN
-#error "None of __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
-#endif
-
 /*
  * Function Prototypes
  */
diff --git a/lib/hexdump.c b/lib/hexdump.c
index ae078536e3f0..940c4eec64e9 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -11,6 +11,26 @@
 #include <linux/printk.h>
 #include <asm/unaligned.h>
 #include <pbl.h>
+#include <asm/byteorder.h>
+
+/*
+ * sanity check. The Linux Kernel defines only one of __LITTLE_ENDIAN and
+ * __BIG_ENDIAN. Endianess can then be tested with #ifdef __xx_ENDIAN. Userspace
+ * always defined both __LITTLE_ENDIAN and __BIG_ENDIAN and byteorder can then
+ * be tested with #if __BYTE_ORDER == __xx_ENDIAN.
+ *
+ * As we tend to use a lot of Kernel code in barebox we use the kernel way of
+ * determing the byte order. Make sure here that architecture code properly
+ * defines it.
+ * An additional benefit of placing this here is that this file is always built.
+ */
+#if defined __LITTLE_ENDIAN && defined __BIG_ENDIAN
+#error "both __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
+#endif
+#if !defined __LITTLE_ENDIAN && !defined __BIG_ENDIAN
+#error "None of __LITTLE_ENDIAN and __BIG_ENDIAN are defined"
+#endif
+
 
 const char hex_asc[] = "0123456789abcdef";
 EXPORT_SYMBOL(hex_asc);
-- 
2.39.5

[PATCH 05/12] include: common.h: move out user interface functions into stdio.h

[Ahmad Fatoum]

These two functions fit nicely with the other functions in stdio.h,
which is already being included by common.h.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h | 4 ----
 include/stdio.h  | 3 +++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/common.h b/include/common.h
index bab03a0444f7..9a0b4768b015 100644
--- a/include/common.h
+++ b/include/common.h
@@ -28,10 +28,6 @@
  */
 void reginfo(void);
 
-char *size_human_readable(unsigned long long size);
-
-int	readline	(const char *prompt, char *buf, int len);
-
 /* common/memsize.c */
 long	get_ram_size  (volatile long *, long);
 
diff --git a/include/stdio.h b/include/stdio.h
index 33ca73a80d18..1ed7e1d3e38b 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -79,6 +79,9 @@ static inline void ctrlc_handled(void)
 
 #endif
 
+char *size_human_readable(unsigned long long size);
+int readline(const char *prompt, char *buf, int len);
+
 #if (!defined(__PBL__) && !defined(CONFIG_CONSOLE_NONE)) || \
 	(defined(__PBL__) && defined(CONFIG_PBL_CONSOLE))
 static inline int puts(const char *s)
-- 
2.39.5

[PATCH 06/12] include: common.h: move out memory option parsing prototypes

[Ahmad Fatoum]

All code using these memory parsing functions already includes getopt.h,
which is the primary file for option parsing.

Therefore, let's move these two functions there as well.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h | 5 -----
 include/getopt.h | 8 ++++++++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/common.h b/include/common.h
index 9a0b4768b015..12961abbf2f5 100644
--- a/include/common.h
+++ b/include/common.h
@@ -31,11 +31,6 @@ void reginfo(void);
 /* common/memsize.c */
 long	get_ram_size  (volatile long *, long);
 
-int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
-		char **sourcefile, char **destfile, int *swab);
-int memcpy_parse_options(int argc, char *argv[], int *sourcefd,
-			 int *destfd, loff_t *count,
-			 int rwsize, int destmode);
 #define RW_BUF_SIZE	(unsigned)4096
 
 #endif	/* __COMMON_H_ */
diff --git a/include/getopt.h b/include/getopt.h
index d4c01e793ad6..aa3b8d8ec6f9 100644
--- a/include/getopt.h
+++ b/include/getopt.h
@@ -8,6 +8,8 @@
 #ifndef __GETOPT_H
 #define __GETOPT_H
 
+#include <linux/types.h>
+
 extern int opterr;
 extern int optind;
 extern int optopt;
@@ -44,4 +46,10 @@ struct getopt_context {
 void getopt_context_store(struct getopt_context *ctx);
 void getopt_context_restore(struct getopt_context *ctx);
 
+int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
+		char **sourcefile, char **destfile, int *swab);
+int memcpy_parse_options(int argc, char *argv[], int *sourcefd,
+			 int *destfd, loff_t *count,
+			 int rwsize, int destmode);
+
 #endif /* __GETOPT_H */
-- 
2.39.5

[PATCH 07/12] include: common.h: move out RW_BUF_SIZE definition

[Ahmad Fatoum]

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h | 2 --
 include/unistd.h | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/common.h b/include/common.h
index 12961abbf2f5..dd0d6548e5db 100644
--- a/include/common.h
+++ b/include/common.h
@@ -31,6 +31,4 @@ void reginfo(void);
 /* common/memsize.c */
 long	get_ram_size  (volatile long *, long);
 
-#define RW_BUF_SIZE	(unsigned)4096
-
 #endif	/* __COMMON_H_ */
diff --git a/include/unistd.h b/include/unistd.h
index b78acbfd737a..f3d2378687e4 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -5,6 +5,8 @@
 #include <linux/types.h>
 #include <fcntl.h>
 
+#define RW_BUF_SIZE	(unsigned)4096
+
 struct stat;
 
 int unlinkat(int dirfd, const char *pathname, int flags);
-- 
2.39.5

[PATCH 08/12] commands: add macro to simplify defining one shot commands

[Ahmad Fatoum]

For development, it can be nice to just be able to call a function as-is
from the command line. Add a helper to facilitate this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/command.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/command.h b/include/command.h
index 26e06077b071..ae8821969c60 100644
--- a/include/command.h
+++ b/include/command.h
@@ -119,4 +119,19 @@ static const __maybe_unused char cmd_##_name##_help[] =
 
 int register_command(struct command *);
 
+#define DEFINE_SIMPLE_COMMAND(func) \
+	struct string_list; \
+	int empty_complete(struct string_list *sl, char *instr); \
+	static int do_simple_command_##func(int argc, char *argv[]) \
+	{ \
+		func(); \
+		return 0; \
+	} \
+	BAREBOX_CMD_START(func) \
+		.cmd		= do_simple_command_##func, \
+		BAREBOX_CMD_DESC("call " # func "()") \
+		BAREBOX_CMD_GROUP(CMD_GRP_INFO) \
+		BAREBOX_CMD_COMPLETE(empty_complete) \
+	BAREBOX_CMD_END
+
 #endif	/* __COMMAND_H */
-- 
2.39.5

[PATCH 09/12] commands: reginfo: make command mpc5xxx-specific

[Ahmad Fatoum]

The reginfo command sounds very generic, but it's actually only used on
the PowerPC MPC5xxx. Move the command there using the new
DEFINE_SIMPLE_COMMAND helper.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/powerpc/mach-mpc5xxx/Kconfig   |  4 ----
 arch/powerpc/mach-mpc5xxx/Makefile  |  2 +-
 arch/powerpc/mach-mpc5xxx/reginfo.c |  4 +++-
 commands/Kconfig                    | 11 -----------
 commands/Makefile                   |  1 -
 commands/reginfo.c                  | 21 ---------------------
 include/common.h                    |  5 -----
 7 files changed, 4 insertions(+), 44 deletions(-)
 delete mode 100644 commands/reginfo.c

diff --git a/arch/powerpc/mach-mpc5xxx/Kconfig b/arch/powerpc/mach-mpc5xxx/Kconfig
index e78c2fa350b1..df6d8c187459 100644
--- a/arch/powerpc/mach-mpc5xxx/Kconfig
+++ b/arch/powerpc/mach-mpc5xxx/Kconfig
@@ -7,10 +7,6 @@ config ARCH_TEXT_BASE
 	default 0x00000000 if RELOCATABLE
 	default 0x01000000 if MACH_PHYCORE_MPC5200B_TINY
 
-config HAS_REGINFO
-	bool
-	default y if ARCH_MPC5200
-
 choice
 	prompt "Select your board"
 
diff --git a/arch/powerpc/mach-mpc5xxx/Makefile b/arch/powerpc/mach-mpc5xxx/Makefile
index 9fc45c08bb31..b9b4cb8e434e 100644
--- a/arch/powerpc/mach-mpc5xxx/Makefile
+++ b/arch/powerpc/mach-mpc5xxx/Makefile
@@ -8,4 +8,4 @@ obj-y				+= traps.o
 obj-y				+= time.o
 extra-y				+= start.o
 obj-$(CONFIG_MPC5200)		+= firmware_sc_task_bestcomm.impl.o
-obj-$(CONFIG_REGINFO)		+= reginfo.o
+obj-$(CONFIG_COMMAND_SUPPORT)	+= reginfo.o
diff --git a/arch/powerpc/mach-mpc5xxx/reginfo.c b/arch/powerpc/mach-mpc5xxx/reginfo.c
index 64816faffc22..1a958b2cefbf 100644
--- a/arch/powerpc/mach-mpc5xxx/reginfo.c
+++ b/arch/powerpc/mach-mpc5xxx/reginfo.c
@@ -4,9 +4,10 @@
 #include <common.h>
 #include <config.h>
 #include <mach/mpc5xxx.h>
+#include <command.h>
 #include <asm/io.h>
 
-void reginfo(void)
+static void reginfo(void)
 {
 	puts ("\nMPC5200 registers\n");
 	printf ("MBAR=%08x\n", CFG_MBAR);
@@ -61,3 +62,4 @@ void reginfo(void)
 	printf ("\tSDRAMCS1: %08X\n",
 		in_be32((void*)MPC5XXX_SDRAM_CS1CFG));
 }
+DEFINE_SIMPLE_COMMAND(reginfo);
diff --git a/commands/Kconfig b/commands/Kconfig
index 4a0486861186..313fa51dcadd 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1,8 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-config REGINFO
-	bool
-
 config COMMAND_SUPPORT
 	bool
 	depends on !SHELL_NONE
@@ -247,14 +244,6 @@ config CMD_ARM_MMUINFO
 	  SuperSection [1]:         0x0
 	  Failure [0]:              0x0
 
-config CMD_REGINFO
-	depends on HAS_REGINFO
-	select REGINFO
-	tristate
-	prompt "reginfo"
-	help
-	  Print register information.
-
 config CMD_BLKSTATS
 	bool
 	depends on BLOCK
diff --git a/commands/Makefile b/commands/Makefile
index ff5d713ca72c..a98c39637cee 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -37,7 +37,6 @@ obj-$(CONFIG_CMD_CAT)		+= cat.o
 obj-$(CONFIG_CMD_MOUNT)		+= mount.o
 obj-$(CONFIG_CMD_UMOUNT)	+= umount.o
 obj-$(CONFIG_CMD_FINDMNT)	+= findmnt.o
-obj-$(CONFIG_CMD_REGINFO)	+= reginfo.o
 obj-$(CONFIG_CMD_CRC)		+= crc.o
 obj-$(CONFIG_CMD_CLEAR)		+= clear.o
 obj-$(CONFIG_CMD_TEST)		+= test.o
diff --git a/commands/reginfo.c b/commands/reginfo.c
deleted file mode 100644
index bd46cf15e7de..000000000000
--- a/commands/reginfo.c
+++ /dev/null
@@ -1,21 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-// SPDX-FileCopyrightText: © 2007 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
-
-/* reginfo.c - print information about SoC specifc registers */
-
-#include <common.h>
-#include <command.h>
-#include <complete.h>
-
-static int do_reginfo(int argc, char *argv[])
-{
-	reginfo();
-	return 0;
-}
-
-BAREBOX_CMD_START(reginfo)
-	.cmd		= do_reginfo,
-	BAREBOX_CMD_DESC("print register information")
-	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
-	BAREBOX_CMD_COMPLETE(empty_complete)
-BAREBOX_CMD_END
diff --git a/include/common.h b/include/common.h
index dd0d6548e5db..1f00baf8067a 100644
--- a/include/common.h
+++ b/include/common.h
@@ -23,11 +23,6 @@
 #include <linux/printk.h>
 #include <barebox-info.h>
 
-/*
- * Function Prototypes
- */
-void reginfo(void);
-
 /* common/memsize.c */
 long	get_ram_size  (volatile long *, long);
 
-- 
2.39.5

Re: [PATCH 09/12] commands: reginfo: make command mpc5xxx-specific

[Sascha Hauer]

On Mon, Oct 14, 2024 at 01:50:37PM +0200, Ahmad Fatoum wrote:
> The reginfo command sounds very generic, but it's actually only used on
> the PowerPC MPC5xxx. Move the command there using the new
> DEFINE_SIMPLE_COMMAND helper.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/powerpc/mach-mpc5xxx/Kconfig   |  4 ----
>  arch/powerpc/mach-mpc5xxx/Makefile  |  2 +-
>  arch/powerpc/mach-mpc5xxx/reginfo.c |  4 +++-
>  commands/Kconfig                    | 11 -----------
>  commands/Makefile                   |  1 -
>  commands/reginfo.c                  | 21 ---------------------
>  include/common.h                    |  5 -----
>  7 files changed, 4 insertions(+), 44 deletions(-)
>  delete mode 100644 commands/reginfo.c
> 
> diff --git a/arch/powerpc/mach-mpc5xxx/Kconfig b/arch/powerpc/mach-mpc5xxx/Kconfig
> index e78c2fa350b1..df6d8c187459 100644
> --- a/arch/powerpc/mach-mpc5xxx/Kconfig
> +++ b/arch/powerpc/mach-mpc5xxx/Kconfig
> @@ -7,10 +7,6 @@ config ARCH_TEXT_BASE
>  	default 0x00000000 if RELOCATABLE
>  	default 0x01000000 if MACH_PHYCORE_MPC5200B_TINY
>  
> -config HAS_REGINFO
> -	bool
> -	default y if ARCH_MPC5200
> -
>  choice
>  	prompt "Select your board"
>  
> diff --git a/arch/powerpc/mach-mpc5xxx/Makefile b/arch/powerpc/mach-mpc5xxx/Makefile
> index 9fc45c08bb31..b9b4cb8e434e 100644
> --- a/arch/powerpc/mach-mpc5xxx/Makefile
> +++ b/arch/powerpc/mach-mpc5xxx/Makefile
> @@ -8,4 +8,4 @@ obj-y				+= traps.o
>  obj-y				+= time.o
>  extra-y				+= start.o
>  obj-$(CONFIG_MPC5200)		+= firmware_sc_task_bestcomm.impl.o
> -obj-$(CONFIG_REGINFO)		+= reginfo.o
> +obj-$(CONFIG_COMMAND_SUPPORT)	+= reginfo.o
> diff --git a/arch/powerpc/mach-mpc5xxx/reginfo.c b/arch/powerpc/mach-mpc5xxx/reginfo.c
> index 64816faffc22..1a958b2cefbf 100644
> --- a/arch/powerpc/mach-mpc5xxx/reginfo.c
> +++ b/arch/powerpc/mach-mpc5xxx/reginfo.c
> @@ -4,9 +4,10 @@
>  #include <common.h>
>  #include <config.h>
>  #include <mach/mpc5xxx.h>
> +#include <command.h>
>  #include <asm/io.h>
>  
> -void reginfo(void)
> +static void reginfo(void)
>  {
>  	puts ("\nMPC5200 registers\n");
>  	printf ("MBAR=%08x\n", CFG_MBAR);
> @@ -61,3 +62,4 @@ void reginfo(void)
>  	printf ("\tSDRAMCS1: %08X\n",
>  		in_be32((void*)MPC5XXX_SDRAM_CS1CFG));
>  }
> +DEFINE_SIMPLE_COMMAND(reginfo);

This is great for development, but not so nice for a broader use in the
tree.

> -BAREBOX_CMD_START(reginfo)
> -	.cmd		= do_reginfo,
> -	BAREBOX_CMD_DESC("print register information")

We are losing the command description and get a "call func reginfo"
instead which is not really an improvement.

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 |

Re: [PATCH 09/12] commands: reginfo: make command mpc5xxx-specific

[Ahmad Fatoum]

Hello Sascha,

On 15.10.24 09:13, Sascha Hauer wrote:
> On Mon, Oct 14, 2024 at 01:50:37PM +0200, Ahmad Fatoum wrote:

>> -void reginfo(void)
>> +static void reginfo(void)
>>  {
>>  	puts ("\nMPC5200 registers\n");
>>  	printf ("MBAR=%08x\n", CFG_MBAR);
>> @@ -61,3 +62,4 @@ void reginfo(void)
>>  	printf ("\tSDRAMCS1: %08X\n",
>>  		in_be32((void*)MPC5XXX_SDRAM_CS1CFG));
>>  }
>> +DEFINE_SIMPLE_COMMAND(reginfo);
> 
> This is great for development, but not so nice for a broader use in the
> tree.

It's meant for one-off things during development and we should at least
have one instance in tree to verify that it still works.

This would be that instance :-)

> 
>> -BAREBOX_CMD_START(reginfo)
>> -	.cmd		= do_reginfo,
>> -	BAREBOX_CMD_DESC("print register information")
> 
> We are losing the command description and get a "call func reginfo"
> instead which is not really an improvement.

That's how it currently looks like:
https://www.barebox.org/doc/latest/commands/info/reginfo.html

And it's confusing, because it's only implemented for one specific
PowerPC machine. I like the new help text more in this instance.

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 |

[PATCH 10/12] include: common.h: move out get_ram_size

[Ahmad Fatoum]

The function get_ram_size() is the last function remaining that's only
defined in common.h. Let's move it to barebox.h instead, so users don't
need to include everything else that's in common.h at the same time.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/barebox.h | 2 ++
 include/common.h  | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/barebox.h b/include/barebox.h
index 741463bc1cf9..09f8c78abbeb 100644
--- a/include/barebox.h
+++ b/include/barebox.h
@@ -32,4 +32,6 @@ enum autoboot_state do_autoboot_countdown(void);
 void __noreturn start_barebox(void);
 void shutdown_barebox(void);
 
+long get_ram_size(volatile long *base, long size);
+
 #endif
diff --git a/include/common.h b/include/common.h
index 1f00baf8067a..16a08fe293af 100644
--- a/include/common.h
+++ b/include/common.h
@@ -23,7 +23,4 @@
 #include <linux/printk.h>
 #include <barebox-info.h>
 
-/* common/memsize.c */
-long	get_ram_size  (volatile long *, long);
-
 #endif	/* __COMMON_H_ */
-- 
2.39.5

[PATCH 11/12] include: align: reword STACK_ALIGN_ARRAY macro parameter for clarity

[Ahmad Fatoum]

Instead of using an ambiguous parameter name 'size' and having a comment
to clarify what's meant by it, just rename it nelems to make clear that
it's the number of elements that should be specified as argument.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/align.h | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/linux/align.h b/include/linux/align.h
index 3b0c60d6e9ae..ff3134db9ea1 100644
--- a/include/linux/align.h
+++ b/include/linux/align.h
@@ -15,12 +15,9 @@
 /*
  * The STACK_ALIGN_ARRAY macro is used to allocate a buffer on the stack that
  * meets a minimum alignment requirement.
- *
- * Note that the size parameter is the number of array elements to allocate,
- * not the number of bytes.
- */
-#define STACK_ALIGN_ARRAY(type, name, size, align)		\
-	char __##name[sizeof(type) * (size) + (align) - 1];	\
-	type *name = (type *)ALIGN((unsigned long)__##name, align)
+  */
+#define STACK_ALIGN_ARRAY(type, name, nelems, align)		\
+	char __##name[sizeof(type) * (nelems) + (align) - 1];	\
+	type *name = (type *)ALIGN((uintptr_t)__##name, align)
 
 #endif
-- 
2.39.5

[PATCH 12/12] ARM: bcm283x: remove common.h include in mbox.h

[Ahmad Fatoum]

Now that common.h is finally just a list of files to include, we can
start removing the inclusion of common.h in other headers and include
only what's actually needed.

Let's start with the raspberry Pi mailbox header and make it (and its
users) only include what's actually needed.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/boards/raspberry-pi/mbox-helpers.c | 1 +
 include/mach/bcm283x/mbox.h                 | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boards/raspberry-pi/mbox-helpers.c b/arch/arm/boards/raspberry-pi/mbox-helpers.c
index 3a76ac2b012f..489eb0c91313 100644
--- a/arch/arm/boards/raspberry-pi/mbox-helpers.c
+++ b/arch/arm/boards/raspberry-pi/mbox-helpers.c
@@ -2,6 +2,7 @@
 // SPDX-FileCopyrightText: 2009 Carlo Caione <carlo@carlocaione.org>
 
 #include <mach/bcm283x/mbox.h>
+#include <linux/printk.h>
 #include "lowlevel.h"
 
 struct msg_get_arm_mem {
diff --git a/include/mach/bcm283x/mbox.h b/include/mach/bcm283x/mbox.h
index cf5143673ab3..fc51b6597560 100644
--- a/include/mach/bcm283x/mbox.h
+++ b/include/mach/bcm283x/mbox.h
@@ -9,7 +9,9 @@
 #ifndef _BCM2835_MBOX_H
 #define _BCM2835_MBOX_H
 
-#include <common.h>
+#include <linux/align.h>
+#include <linux/types.h>
+#include <linux/string.h>
 
 #include <mach/bcm283x/platform.h>
 
-- 
2.39.5

Re: [PATCH 00/12] include: common.h: make it include only headers

[Sascha Hauer]

On Mon, 14 Oct 2024 13:50:28 +0200, Ahmad Fatoum wrote:
> Symbols exclusively defined in common.h are a problem, because other
> headers that require them will need to include a lot of extra baggage,
> which in the worst case can lead to cyclic dependencies and in every
> case leads to longer compile times.
> 
> This series prepare for removing common.h in other headers by moving
> everything it contains apart from #includes into more fitting existing
> headers that are already being included.
> 
> [...]

Applied, thanks!

[01/12] include: common.h: move barebox startup functions into separate header
        https://git.pengutronix.de/cgit/barebox/commit/?id=e723ea2ed886 (link may not be stable)
[02/12] include: common.h: move ctrlc() functions into stdio.h
        https://git.pengutronix.de/cgit/barebox/commit/?id=0421e3c77def (link may not be stable)
[03/12] include: common.h: move out integer string parsing functions
        https://git.pengutronix.de/cgit/barebox/commit/?id=af51386c2627 (link may not be stable)
[04/12] include: common.h: move out endianness macro sanity check
        https://git.pengutronix.de/cgit/barebox/commit/?id=2a8222c22c2d (link may not be stable)
[05/12] include: common.h: move out user interface functions into stdio.h
        https://git.pengutronix.de/cgit/barebox/commit/?id=b3c618880ed5 (link may not be stable)
[06/12] include: common.h: move out memory option parsing prototypes
        https://git.pengutronix.de/cgit/barebox/commit/?id=7fa8ca6a4094 (link may not be stable)
[07/12] include: common.h: move out RW_BUF_SIZE definition
        https://git.pengutronix.de/cgit/barebox/commit/?id=1b33d75d4b07 (link may not be stable)
[08/12] commands: add macro to simplify defining one shot commands
        (no commit info)
[09/12] commands: reginfo: make command mpc5xxx-specific
        (no commit info)
[10/12] include: common.h: move out get_ram_size
        (no commit info)
[11/12] include: align: reword STACK_ALIGN_ARRAY macro parameter for clarity
        (no commit info)
[12/12] ARM: bcm283x: remove common.h include in mbox.h
        (no commit info)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH 00/12] include: common.h: make it include only headers

[Sascha Hauer]

On Tue, Oct 15, 2024 at 09:10:29AM +0200, Sascha Hauer wrote:
> 
> On Mon, 14 Oct 2024 13:50:28 +0200, Ahmad Fatoum wrote:
> > Symbols exclusively defined in common.h are a problem, because other
> > headers that require them will need to include a lot of extra baggage,
> > which in the worst case can lead to cyclic dependencies and in every
> > case leads to longer compile times.
> > 
> > This series prepare for removing common.h in other headers by moving
> > everything it contains apart from #includes into more fitting existing
> > headers that are already being included.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [01/12] include: common.h: move barebox startup functions into separate header
>         https://git.pengutronix.de/cgit/barebox/commit/?id=e723ea2ed886 (link may not be stable)
> [02/12] include: common.h: move ctrlc() functions into stdio.h
>         https://git.pengutronix.de/cgit/barebox/commit/?id=0421e3c77def (link may not be stable)
> [03/12] include: common.h: move out integer string parsing functions
>         https://git.pengutronix.de/cgit/barebox/commit/?id=af51386c2627 (link may not be stable)
> [04/12] include: common.h: move out endianness macro sanity check
>         https://git.pengutronix.de/cgit/barebox/commit/?id=2a8222c22c2d (link may not be stable)
> [05/12] include: common.h: move out user interface functions into stdio.h
>         https://git.pengutronix.de/cgit/barebox/commit/?id=b3c618880ed5 (link may not be stable)
> [06/12] include: common.h: move out memory option parsing prototypes
>         https://git.pengutronix.de/cgit/barebox/commit/?id=7fa8ca6a4094 (link may not be stable)
> [07/12] include: common.h: move out RW_BUF_SIZE definition
>         https://git.pengutronix.de/cgit/barebox/commit/?id=1b33d75d4b07 (link may not be stable)
> [08/12] commands: add macro to simplify defining one shot commands
>         (no commit info)
> [09/12] commands: reginfo: make command mpc5xxx-specific
>         (no commit info)
> [10/12] include: common.h: move out get_ram_size
>         (no commit info)
> [11/12] include: align: reword STACK_ALIGN_ARRAY macro parameter for clarity
>         (no commit info)
> [12/12] ARM: bcm283x: remove common.h include in mbox.h
>         (no commit info)

Hm, I assumed b4 ty would automatically send a "partly applied" message.
I've seen this with others using b4 ty apparently. I don't know what
went wrong here.

In fact I applied patches 1-7.

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 |

Re: [PATCH 00/12] include: common.h: make it include only headers

[Konstantin Ryabitsev]

On Tue, Oct 15, 2024 at 09:16:17AM +0200, Sascha Hauer wrote:
> Hm, I assumed b4 ty would automatically send a "partly applied" message.
> I've seen this with others using b4 ty apparently. I don't know what
> went wrong here.

For b4 to know that it's what you intended, you have to apply the series with
a --cherry-pick command, e.g.:

b4 am -P 1-7 [msgid]

Since you retrieved the entire series, b4 assumes
that we just can't find the commits associated with the patches you retrieved.

We will have a better review and confirm screen in the future that should
hopefully help avoid this situation.

-K

Re: [PATCH 00/12] include: common.h: make it include only headers

[Sascha Hauer]

Hi Konstantin,

On Tue, Oct 15, 2024 at 09:35:01AM -0400, Konstantin Ryabitsev wrote:
> On Tue, Oct 15, 2024 at 09:16:17AM +0200, Sascha Hauer wrote:
> > Hm, I assumed b4 ty would automatically send a "partly applied" message.
> > I've seen this with others using b4 ty apparently. I don't know what
> > went wrong here.
> 
> For b4 to know that it's what you intended, you have to apply the series with
> a --cherry-pick command, e.g.:
> 
> b4 am -P 1-7 [msgid]
> 
> Since you retrieved the entire series, b4 assumes
> that we just can't find the commits associated with the patches you retrieved.

Thanks for clarifying. Indeed I applied the whole series and just
dropped the patches I didn't want to have afterwards. I should have
applied only the desired patches in the first place.

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 |