[PATCH] net.c: Don't forget about the first fragment.

[Christian Melki]

It's possible to request very large messages using
the current code base. F.ex. UDP datagrams with the tftp client.
The tftp servers will happily reply with fragmented IP frames.
All these frame parts need to be dropped as BB currently doesn't
do fragment reassembly.

The current check was for fragment offsets only (0x1fff).
But the first frame has fragment offset 0 and would slip through
this check. That could result in a seemingly OK frame
for the tftp client, but with broken data.

Add check for the MF (More Fragments) flag. Should cover the
first packet too.

Signed-off-by: Christian Melki 
---
 net/net.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 19161d2e82..b842041d59 100644
--- a/net/net.c
+++ b/net/net.c
@@ -678,7 +678,12 @@ static int net_handle_ip(struct eth_device *edev, unsigned 
char *pkt, int len)
if ((ip->hl_v & 0xf0) != 0x40)
goto bad;
 
-   if (ip->frag_off & htons(0x1fff)) /* Can't deal w/ fragments */
+   /* Can't deal w/ fragments.
+* Ether a fragment offset (13 bits), or
+* MF (More Fragments) from frag. flags (3 bits).
+* MF - because first fragment has fragment offset 0
+*/
+   if (ip->frag_off & htons(0x3fff)) 
goto bad;
if (!net_checksum_ok((unsigned char *)ip, sizeof(struct iphdr)))
goto bad;
-- 
2.34.1

[PATCH v3 2/2] commands: test: add based support for bash-test style

[Marco Felsch]

Add bash-style test [[ expression ]] to support pattern matching like:

  - [[ "foo1" == "foo*" ]]
  - [[ "foo1" != "bar*" ]]

For more information see man 1 bash.

Signed-off-by: Marco Felsch 
---
v3:
 - replace pr_* with printf
v2:
 - drop addittional Kconfig switch
 - fix FNMATCH select
 - drop conditional compilation
 - adapt commit message

 commands/Kconfig |  6 --
 commands/test.c  | 33 +++--
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index 7ca7b56fa5..17ad5ff87c 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1208,15 +1208,17 @@ config CMD_SLEEP
 config CMD_TEST
tristate
depends on SHELL_HUSH
+   select FNMATCH
default y
prompt "test"
help
- Minimal test command like in /bin/sh
+ Minimal test command like in /bin/sh with support for bash-style
+ [[ expression ]] tests as well.
 
  Usage: test [EXPR]
 
  Options:
- !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, 
-e,
+ !, =, ==, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, 
-d, -e,
  -f, -L; see 'man test' on your PC for more information.
 
 config CMD_TRUE
diff --git a/commands/test.c b/commands/test.c
index c1b84c42ef..1130bf2e5b 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -9,11 +9,13 @@
  */
 #include 
 #include 
+#include 
 #include 
 #include 
 
 typedef enum {
OPT_EQUAL,
+   OPT_EQUAL_BASH,
OPT_NOT_EQUAL,
OPT_ARITH_EQUAL,
OPT_ARITH_NOT_EQUAL,
@@ -36,6 +38,7 @@ typedef enum {
 
 static char *test_options[] = {
[OPT_EQUAL] = "=",
+   [OPT_EQUAL_BASH]= "==",
[OPT_NOT_EQUAL] = "!=",
[OPT_ARITH_EQUAL]   = "-eq",
[OPT_ARITH_NOT_EQUAL]   = "-ne",
@@ -67,18 +70,35 @@ static int parse_opt(const char *opt)
return -1;
 }
 
+static int string_comp(const char *left_op, const char *right_op, bool 
bash_test)
+{
+   if (bash_test)
+   return fnmatch(right_op, left_op, 0);
+
+   return strcmp(left_op, right_op);
+}
+
 static int do_test(int argc, char *argv[])
 {
char **ap;
int left, adv, expr, last_expr, neg, last_cmp, opt, zero;
ulong a, b;
struct stat statbuf;
+   bool bash_test = false;
 
if (*argv[0] == '[') {
argc--;
-   if (*argv[argc] != ']') {
-   printf("[: missing `]'\n");
-   return 1;
+   if (!strncmp(argv[0], "[[", 2)) {
+   if (strncmp(argv[argc], "]]", 2) != 0) {
+   printf("[[: missing `]]'\n");
+   return 1;
+   }
+   bash_test = true;
+   } else {
+   if (*argv[argc] != ']') {
+   printf("[: missing `]'\n");
+   return 1;
+   }
}
}
 
@@ -183,10 +203,11 @@ static int do_test(int argc, char *argv[])
b = simple_strtol(ap[2], NULL, 0);
switch (parse_opt(ap[1])) {
case OPT_EQUAL:
-   expr = strcmp(ap[0], ap[2]) == 0;
+   case OPT_EQUAL_BASH:
+   expr = string_comp(ap[0], ap[2], bash_test) == 
0;
break;
case OPT_NOT_EQUAL:
-   expr = strcmp(ap[0], ap[2]) != 0;
+   expr = string_comp(ap[0], ap[2], bash_test) != 
0;
break;
case OPT_ARITH_EQUAL:
expr = a == b;
@@ -233,7 +254,7 @@ static int do_test(int argc, char *argv[])
return expr;
 }
 
-static const char * const test_aliases[] = { "[", NULL};
+static const char * const test_aliases[] = { "[", "[[", NULL};
 
 BAREBOX_CMD_HELP_START(test)
 BAREBOX_CMD_HELP_TEXT("Options:")
-- 
2.39.2

[PATCH v3 1/2] commands: test: simplify argv handling

[Marco Felsch]

Decrement argc first before check the closing ']' to avoid the
*argv[argc - 1]. No functional change.

Signed-off-by: Marco Felsch 
---
v3:
- no changes
v2:
- no changes

 commands/test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commands/test.c b/commands/test.c
index c845cec017..c1b84c42ef 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -75,11 +75,11 @@ static int do_test(int argc, char *argv[])
struct stat statbuf;
 
if (*argv[0] == '[') {
-   if (*argv[argc - 1] != ']') {
+   argc--;
+   if (*argv[argc] != ']') {
printf("[: missing `]'\n");
return 1;
}
-   argc--;
}
 
/* args? */
-- 
2.39.2

Re: [RFC 0/4] Run barebox on sc6531e-based feature phone

[Sascha Hauer]

Hi Antony,

This has stayed unanswered for some time.

I am not sure what I should do with it. In the current form I think it's
a bit too basic to be useful. Maybe we can add it once the LCD and
Keypad driver is ready?

Sascha

On Sat, Jun 10, 2023 at 01:32:16PM +0300, Antony Pavlov wrote:
> This patch series adds initial support for SC6531E chip.
> 
> SC6431E chip is specially designed solution for creating
> feature phones. It contains
> 
>   * ARM926EJ-S core (up to 208 MHz)
>   * embedded PSRAM
>   * peripherals: USB, SPI, UART, IIS, PCM, I2C, keypad, LCD
>   * SIM card and GSM/GPRS/FM/BT stuff
> 
> This work is based on FPDoom, see [1], [2], [3] for details.
> 
> At the moment only timer and debug_ll output via USB
> are supported.
> 
> After adding lcd and keypad driver running bareDOOM [4]
> on SC6531E will be possible.
> 
>   [1] https://github.com/ilyakurdyukov/fpdoom
>   [2] https://habr.com/ru/articles/706766/
>   [3] https://www.youtube.com/watch?v=tln_Iace1O8
>   [4] https://github.com/a3f/bareDOOM
> 
> Antony Pavlov (4):
>   clocksource: add sc6531e driver
>   ARM: add sc6531e and F+ Ezzy 4 phone support
>   sc6531e: add debug_ll support
>   Documentation: add sc6531e instructions
> 
>  Documentation/boards/sc6531e.rst  | 187 +
>  arch/arm/Kconfig  |  10 +
>  arch/arm/Makefile |   1 +
>  arch/arm/boards/Makefile  |   1 +
>  arch/arm/boards/ezzy-4/Makefile   |   6 +
>  arch/arm/boards/ezzy-4/env/init/automount |  27 ++
>  arch/arm/boards/ezzy-4/lowlevel.c |  19 +
>  arch/arm/boards/ezzy-4/usbio.c| 449 ++
>  arch/arm/configs/ezzy-4_defconfig |  49 +++
>  arch/arm/cpu/uncompress.c |   7 +
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/ezzy-4.dts   |  22 ++
>  arch/arm/include/asm/debug_ll.h   |   2 +
>  arch/arm/mach-sc6531e/Kconfig |  17 +
>  arch/arm/mach-sc6531e/Makefile|   3 +
>  drivers/clocksource/Makefile  |   1 +
>  drivers/clocksource/timer-sc6531e.c   |  70 
>  include/mach/sc6531e/debug_ll.h   |  17 +
>  18 files changed, 889 insertions(+)
>  create mode 100644 Documentation/boards/sc6531e.rst
>  create mode 100644 arch/arm/boards/ezzy-4/Makefile
>  create mode 100644 arch/arm/boards/ezzy-4/env/init/automount
>  create mode 100644 arch/arm/boards/ezzy-4/lowlevel.c
>  create mode 100644 arch/arm/boards/ezzy-4/usbio.c
>  create mode 100644 arch/arm/configs/ezzy-4_defconfig
>  create mode 100644 arch/arm/dts/ezzy-4.dts
>  create mode 100644 arch/arm/mach-sc6531e/Kconfig
>  create mode 100644 arch/arm/mach-sc6531e/Makefile
>  create mode 100644 drivers/clocksource/timer-sc6531e.c
>  create mode 100644 include/mach/sc6531e/debug_ll.h
> 
> -- 
> 2.39.0
> 
> 

-- 
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- |

Re: [PATCH v2 2/3] commands: test: replace printf with pr_err

[Marco Felsch]

On 23-07-04, Sascha Hauer wrote:
> On Tue, Jul 04, 2023 at 01:29:20PM +0200, Marco Felsch wrote:
> > Replace the printf() with pr_err() to support:
> >  - color formated prints
> >  - logbuffer storing (dmesg)
> >  - loglevels
> > 
> > Signed-off-by: Marco Felsch 
> > ---
> > v2:
> >  - new patch
> > 
> >  commands/test.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/commands/test.c b/commands/test.c
> > index c1b84c42ef..5478011185 100644
> > --- a/commands/test.c
> > +++ b/commands/test.c
> > @@ -77,7 +77,7 @@ static int do_test(int argc, char *argv[])
> > if (*argv[0] == '[') {
> > argc--;
> > if (*argv[argc] != ']') {
> > -   printf("[: missing `]'\n");
> > +   pr_err("[: missing `]'\n");
> 
> Logging functions pr_* and dev_* should be used for core and driver
> messages, commands should use printf. The line between logging functions
> and commands is not always clear in barebox, but here it is.

Okay, thanks for clarification. I will drop the patch and send a v3.

Regards,
  Marco

> 
> 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- |
> 

Re: [PATCH 1/2] fixup! ARM: i.MX8MP: Add DEBIX SOM A and SOM A I/O Board support

[Sascha Hauer]

On Tue, Jul 04, 2023 at 12:40:47PM +0200, Marco Felsch wrote:
> ---
>  arch/arm/mach-imx/Kconfig | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 4c93c1244a..4af281d238 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -664,7 +664,7 @@ config MACH_PHYTEC_SOM_IMX8MQ
>   select MCI_IMX_ESDHC_PBL
>  
>  config MACH_POLYHEX_DEBIX
> - bool "Polyhex DEBIX Model-A/B (i.MX8MP) Board"
> + bool "Polyhex DEBIX i.MX8MP Boards"
>   select ARCH_IMX8MP
>   depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>   depends on HAVE_FIRMWARE_IMX8MP_ATF
> @@ -674,6 +674,8 @@ config MACH_POLYHEX_DEBIX
>   select MCI_IMX_ESDHC_PBL
>   select IMX8M_DRAM
>   select I2C_IMX_EARLY
> + help
> +   Support for DEBIX Model-A/B and SOM A + SOM A I/O board
>  
>  config MACH_PROTONIC_IMX8M
>   bool "Protonic-Holland i.MX8Mx based boards"
> -- 
> 2.39.2
> 
> 
> 

-- 
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- |

Re: [PATCH v2 2/3] commands: test: replace printf with pr_err

[Sascha Hauer]

On Tue, Jul 04, 2023 at 01:29:20PM +0200, Marco Felsch wrote:
> Replace the printf() with pr_err() to support:
>  - color formated prints
>  - logbuffer storing (dmesg)
>  - loglevels
> 
> Signed-off-by: Marco Felsch 
> ---
> v2:
>  - new patch
> 
>  commands/test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/commands/test.c b/commands/test.c
> index c1b84c42ef..5478011185 100644
> --- a/commands/test.c
> +++ b/commands/test.c
> @@ -77,7 +77,7 @@ static int do_test(int argc, char *argv[])
>   if (*argv[0] == '[') {
>   argc--;
>   if (*argv[argc] != ']') {
> - printf("[: missing `]'\n");
> + pr_err("[: missing `]'\n");

Logging functions pr_* and dev_* should be used for core and driver
messages, commands should use printf. The line between logging functions
and commands is not always clear in barebox, but here it is.

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- |

[PATCH v2 2/3] commands: test: replace printf with pr_err

[Marco Felsch]

Replace the printf() with pr_err() to support:
 - color formated prints
 - logbuffer storing (dmesg)
 - loglevels

Signed-off-by: Marco Felsch 
---
v2:
 - new patch

 commands/test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commands/test.c b/commands/test.c
index c1b84c42ef..5478011185 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -77,7 +77,7 @@ static int do_test(int argc, char *argv[])
if (*argv[0] == '[') {
argc--;
if (*argv[argc] != ']') {
-   printf("[: missing `]'\n");
+   pr_err("[: missing `]'\n");
return 1;
}
}
@@ -213,7 +213,7 @@ static int do_test(int argc, char *argv[])
}
 
if (left < adv) {
-   printf("test: failed to parse arguments\n");
+   pr_err("test: failed to parse arguments\n");
return 1;
}
 
-- 
2.39.2

[PATCH v2 3/3] commands: test: add based support for bash-test style

[Marco Felsch]

Add bash-style test [[ expression ]] to support pattern matching like:

  - [[ "foo1" == "foo*" ]]
  - [[ "foo1" != "bar*" ]]

For more information see man 1 bash.

Signed-off-by: Marco Felsch 
---
v2:
 - drop addittional Kconfig switch
 - fix FNMATCH select
 - drop conditional compilation
 - adapt commit message

 commands/Kconfig |  6 --
 commands/test.c  | 33 +++--
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index 7ca7b56fa5..17ad5ff87c 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1208,15 +1208,17 @@ config CMD_SLEEP
 config CMD_TEST
tristate
depends on SHELL_HUSH
+   select FNMATCH
default y
prompt "test"
help
- Minimal test command like in /bin/sh
+ Minimal test command like in /bin/sh with support for bash-style
+ [[ expression ]] tests as well.
 
  Usage: test [EXPR]
 
  Options:
- !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, 
-e,
+ !, =, ==, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, 
-d, -e,
  -f, -L; see 'man test' on your PC for more information.
 
 config CMD_TRUE
diff --git a/commands/test.c b/commands/test.c
index 5478011185..b62ef2c65c 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -9,11 +9,13 @@
  */
 #include 
 #include 
+#include 
 #include 
 #include 
 
 typedef enum {
OPT_EQUAL,
+   OPT_EQUAL_BASH,
OPT_NOT_EQUAL,
OPT_ARITH_EQUAL,
OPT_ARITH_NOT_EQUAL,
@@ -36,6 +38,7 @@ typedef enum {
 
 static char *test_options[] = {
[OPT_EQUAL] = "=",
+   [OPT_EQUAL_BASH]= "==",
[OPT_NOT_EQUAL] = "!=",
[OPT_ARITH_EQUAL]   = "-eq",
[OPT_ARITH_NOT_EQUAL]   = "-ne",
@@ -67,18 +70,35 @@ static int parse_opt(const char *opt)
return -1;
 }
 
+static int string_comp(const char *left_op, const char *right_op, bool 
bash_test)
+{
+   if (bash_test)
+   return fnmatch(right_op, left_op, 0);
+
+   return strcmp(left_op, right_op);
+}
+
 static int do_test(int argc, char *argv[])
 {
char **ap;
int left, adv, expr, last_expr, neg, last_cmp, opt, zero;
ulong a, b;
struct stat statbuf;
+   bool bash_test = false;
 
if (*argv[0] == '[') {
argc--;
-   if (*argv[argc] != ']') {
-   pr_err("[: missing `]'\n");
-   return 1;
+   if (!strncmp(argv[0], "[[", 2)) {
+   if (strncmp(argv[argc], "]]", 2) != 0) {
+   pr_err("[[: missing `]]'\n");
+   return 1;
+   }
+   bash_test = true;
+   } else {
+   if (*argv[argc] != ']') {
+   pr_err("[: missing `]'\n");
+   return 1;
+   }
}
}
 
@@ -183,10 +203,11 @@ static int do_test(int argc, char *argv[])
b = simple_strtol(ap[2], NULL, 0);
switch (parse_opt(ap[1])) {
case OPT_EQUAL:
-   expr = strcmp(ap[0], ap[2]) == 0;
+   case OPT_EQUAL_BASH:
+   expr = string_comp(ap[0], ap[2], bash_test) == 
0;
break;
case OPT_NOT_EQUAL:
-   expr = strcmp(ap[0], ap[2]) != 0;
+   expr = string_comp(ap[0], ap[2], bash_test) != 
0;
break;
case OPT_ARITH_EQUAL:
expr = a == b;
@@ -233,7 +254,7 @@ static int do_test(int argc, char *argv[])
return expr;
 }
 
-static const char * const test_aliases[] = { "[", NULL};
+static const char * const test_aliases[] = { "[", "[[", NULL};
 
 BAREBOX_CMD_HELP_START(test)
 BAREBOX_CMD_HELP_TEXT("Options:")
-- 
2.39.2

[PATCH v2 1/3] commands: test: simplify argv handling

[Marco Felsch]

Decrement argc first before check the closing ']' to avoid the
*argv[argc - 1]. No functional change.

Signed-off-by: Marco Felsch 
---
 commands/test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commands/test.c b/commands/test.c
index c845cec017..c1b84c42ef 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -75,11 +75,11 @@ static int do_test(int argc, char *argv[])
struct stat statbuf;
 
if (*argv[0] == '[') {
-   if (*argv[argc - 1] != ']') {
+   argc--;
+   if (*argv[argc] != ']') {
printf("[: missing `]'\n");
return 1;
}
-   argc--;
}
 
/* args? */
-- 
2.39.2

[PATCH 1/2] fixup! ARM: i.MX8MP: Add DEBIX SOM A and SOM A I/O Board support

[Marco Felsch]

---
 arch/arm/mach-imx/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 4c93c1244a..4af281d238 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -664,7 +664,7 @@ config MACH_PHYTEC_SOM_IMX8MQ
select MCI_IMX_ESDHC_PBL
 
 config MACH_POLYHEX_DEBIX
-   bool "Polyhex DEBIX Model-A/B (i.MX8MP) Board"
+   bool "Polyhex DEBIX i.MX8MP Boards"
select ARCH_IMX8MP
depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
depends on HAVE_FIRMWARE_IMX8MP_ATF
@@ -674,6 +674,8 @@ config MACH_POLYHEX_DEBIX
select MCI_IMX_ESDHC_PBL
select IMX8M_DRAM
select I2C_IMX_EARLY
+   help
+ Support for DEBIX Model-A/B and SOM A + SOM A I/O board
 
 config MACH_PROTONIC_IMX8M
bool "Protonic-Holland i.MX8Mx based boards"
-- 
2.39.2

[PATCH 2/2] ARM64: configs: multi_v8_defconfig: enable onboard usb hub support

[Marco Felsch]

Enable the onboard usb hub support since it is at least used on the
DEBIX Model-A i.MX8MP board.

Signed-off-by: Marco Felsch 
---
 arch/arm/configs/multi_v8_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v8_defconfig 
b/arch/arm/configs/multi_v8_defconfig
index d30158d7f8..2ed1c7e27f 100644
--- a/arch/arm/configs/multi_v8_defconfig
+++ b/arch/arm/configs/multi_v8_defconfig
@@ -177,6 +177,7 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_SERIAL=y
 CONFIG_USB_GADGET_FASTBOOT=y
 CONFIG_USB_GADGET_MASS_STORAGE=y
+CONFIG_USB_ONBOARD_HUB=y
 CONFIG_VIDEO=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_DRIVER_VIDEO_BOCHS_PCI=y
-- 
2.39.2

Re: [PATCH 2/2] commands: test: add based support for bash-test style

[Marco Felsch]

On 23-07-04, Sascha Hauer wrote:
> On Tue, Jul 04, 2023 at 10:32:22AM +0200, Marco Felsch wrote:
> > Hi Ahmad,
> > 
> > On 23-07-04, Ahmad Fatoum wrote:
> > > On 04.07.23 00:58, Marco Felsch wrote:
> > > > Add [[ expression ]] support which allow pattern matching like:
> > > > 
> > > >   - [[ "foo1" == "foo*" ]]
> > > >   - [[ "foo" != "bar" ]]
> > > > 
> > > > Signed-off-by: Marco Felsch 
> > > > ---
> > > >  commands/Kconfig | 13 +
> > > >  commands/test.c  | 39 ++-
> > > >  2 files changed, 47 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/commands/Kconfig b/commands/Kconfig
> > > > index 4d3ff631a8..615e96aa9d 100644
> > > > --- a/commands/Kconfig
> > > > +++ b/commands/Kconfig
> > > > @@ -1219,6 +1219,19 @@ config CMD_TEST
> > > >   !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, 
> > > > -n, -d, -e,
> > > >   -f, -L; see 'man test' on your PC for more 
> > > > information.
> > > >  
> > > > +if CMD_TEST
> > > > +
> > > > +config CMD_TEST_BASH_COMP
> > > > +   tristate
> > > > +   select CONFIG_FNMATCH
> > > 
> > > select FNMATCH, no CONFIG_
> > 
> > Right.
> > 
> > > Does this really need its own symbol? Can't you just use CONFIG_GLOB
> > > and amend the help text?
> > 
> > The implementation was inspired by busybox and they use fnmatch.
> > Checking glob.c I don't see any difference, therefore I would keep the
> > fnmatch.
> 
> glob() is the wrong function anyway. glob() expands patterns to existing
> files. You want to do pattern matching only not looking into the VFS.
> fnmatch() is the right function here.
> 
> Besides, glob() uses fnmatch(), so by using glob() you would add even
> more code.
> 
> > 
> > I wasn't sure if every user need the bash(ism) support and sometimes
> > bootloader partitions are really small due to legacy reasons. Therefore
> > I went this way to let the user the choice to enable/disable the
> > support.
> 
> You can't do much in barebox without globalvar support which already
> uses fnmatch(), so it's likely enabled anyway.

Ah right, I will drop it then.

Thanks,
Marco

> 
> Sascha
> 
> > > > +   return fnmatch(right_op, left_op, 0);
> > > > +#endif
> > > 
> > > Scripts that execute differently depending on a barebox config option
> > > is not good user experience. I think it would be better if [[ is an
> > > error if support is missing.
> > 
> > I was thinking about a warning which is printed once you use [[ and
> > didn't enabled it before. Then I read man bash again and [[ supports
> > '=' as well which equals the [ function. Therefore I dropped the
> > warning.
> > 
> > > Also can't the #ifdef be replaced with a IS_ENABLED()
> > 
> > I don't think so, since the fnmatch is added conditional to keep the
> > code base the same if not enabled.
> 
> Use IS_ENABLED(). The linker will discard unused functions for you. We
> use this all over the 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- |
> 

Re: [PATCH] Documentation: use current link to bootloader spec spec

[Sascha Hauer]

On Thu, Jun 29, 2023 at 08:52:25AM +0200, Ahmad Fatoum wrote:
> Both the Freedesktop and the systemd links are outdated, so replace them
> with the current home of the spec.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  Documentation/talks.rst  | 2 +-
>  Documentation/user/booting-linux.rst | 2 +-
>  common/Kconfig   | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/Documentation/talks.rst b/Documentation/talks.rst
> index 5f1af599316b..5206d649a2a7 100644
> --- a/Documentation/talks.rst
> +++ b/Documentation/talks.rst
> @@ -100,7 +100,7 @@ Sascha Hauer, Embedded Linux Conference - Europe 2013
>  `[slides] 
> `__
>  `[video] `__
>  
> -  The freedesktop.org bootloader specification and its support in barebox.
> +  The bootloader specification and its support in barebox.
>  
>  Barebox Bootloader
>  --
> diff --git a/Documentation/user/booting-linux.rst 
> b/Documentation/user/booting-linux.rst
> index 6fa8cd911e06..0ba6229797cd 100644
> --- a/Documentation/user/booting-linux.rst
> +++ b/Documentation/user/booting-linux.rst
> @@ -168,7 +168,7 @@ Bootloader Spec
>  
>  barebox supports booting according to the bootloader spec:
>  
> -https://systemd.io/BOOT_LOADER_SPECIFICATION/
> +https://uapi-group.org/specifications/specs/boot_loader_specification/
>  
>  It follows another philosophy than the :ref:`boot_entries`. With Boot Entries
>  booting is completely configured in the bootloader. Bootloader Spec Entries
> diff --git a/common/Kconfig b/common/Kconfig
> index 3a57e16b3be6..6bcb2dac5b7f 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -684,8 +684,8 @@ config BLSPEC
>   bool
>   prompt "Support bootloader spec"
>   help
> -   Enable this to let barebox support the Freedesktop bootloader spec,
> -   see: http://www.freedesktop.org/wiki/Specifications/BootLoaderSpec/
> +   Enable this to let barebox support the UAPI bootloader spec,
> +   see: 
> https://uapi-group.org/specifications/specs/boot_loader_specification/
> The bootloader spec is a standard interface between the bootloader
> and the kernel. It allows the bootloader to discover boot options
> on a device and it allows the Operating System to install / update
> -- 
> 2.39.2
> 
> 
> 

-- 
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- |

Re: [PATCH] ARM: zynq: get ps_clk_rate from dt

[Sascha Hauer]

On Mon, Jun 26, 2023 at 12:37:46PM +0200, Steffen Trumtrar wrote:
> From: "Assmann Kai (BEG/EMS1)" 
> 
> This adds a barebox-specific binding to overwrite the PS clock
> frequency.
> 
> Currently the ps_clk_rate is locked to 33.3MHz. Introduce a devicetree
> property "ps-clock-frequency" that specifies this clock.
> 
> If the property is found, overwrite ps_clk_rate otherwise stay at the
> default 33.3MHz
> 
> Signed-off-by: Assmann Kai (BEG/MSD-NE2) 
> Signed-off-by: Steffen Trumtrar 
> ---

Applied, thanks

Sascha

> 
> Notes:
> changes since v1:
>  - reword commit message
>  - add binding rst
>  - change type of ps_clk_frequency
> 
>  .../devicetree/bindings/clocks/xlnx,ps7-clkc.rst   | 10 ++
>  drivers/clk/zynq/clkc.c|  4 +++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/clocks/xlnx,ps7-clkc.rst
> 
> diff --git a/Documentation/devicetree/bindings/clocks/xlnx,ps7-clkc.rst 
> b/Documentation/devicetree/bindings/clocks/xlnx,ps7-clkc.rst
> new file mode 100644
> index 00..523b0cf56e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clocks/xlnx,ps7-clkc.rst
> @@ -0,0 +1,10 @@
> +Xilinx PS7 clkc
> +===
> +
> +In addition to the upstream bindings, following properties are understood:
> +
> +Optional properties:
> +
> +- ``ps-clock-frequency`` : Overrides the ps clock frequency set by the 
> driver.
> +  Per default the clock is set to 33.3MHz. When this property is set, the 
> frequency
> +  is overwritten by the devicetree property.
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index 1219e25f7a..d6de583e32 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -368,7 +368,7 @@ static int zynq_clock_probe(struct device *dev)
>  {
>   struct resource *iores;
>   void __iomem *clk_base;
> - unsigned long ps_clk_rate = 3330;
> + u32 ps_clk_rate = 3330;
>   resource_size_t slcr_offset = 0;
>  
>   iores = dev_get_resource(dev, IORESOURCE_MEM, 0);
> @@ -390,6 +390,8 @@ static int zynq_clock_probe(struct device *dev)
>   slcr_offset = parent_res->start;
>   }
>  
> + of_property_read_u32(dev->device_node, "ps-clk-frequency", 
> _clk_rate);
> +
>   iores = request_iomem_region(dev_name(dev), iores->start + slcr_offset,
>iores->end + slcr_offset);
>   if (IS_ERR(iores))
> -- 
> 2.41.0
> 
> 
> 

-- 
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- |

v2023.07.1

[Sascha Hauer]

Hi All,

There is a last minute bug in the v2023.07.0 Release, so here is
a stable tag for that version.

Firmware files compiled into the barebox image are not properly
aligned in the binary resulting in corrupted firmware files, so
when your image needs firmware compiled in, please use v2023.07.1
instead.

Sascha


Ahmad Fatoum (2):
  firmware: fix alignment of firmware images
  firmware: use portable newline escape

Sascha Hauer (1):
  Release v2023.07.1


-- 
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- |

Re: [PATCH 1/2] soc: imx: featctrl: add help text for CONFIG_IMX8M_FEATCTRL

[Sascha Hauer]

On Mon, Jul 03, 2023 at 07:20:53PM +0200, Ahmad Fatoum wrote:
> There's no equivalent Linux driver, so a short help text is certainly in
> order. While at it, replace spaces with tabs for the prompt line.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/soc/imx/Kconfig | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig
> index 06513c02da2f..b11db718c2bc 100644
> --- a/drivers/soc/imx/Kconfig
> +++ b/drivers/soc/imx/Kconfig
> @@ -8,9 +8,12 @@ config IMX_GPCV2_PM_DOMAINS
>   default y if ARCH_IMX7 || ARCH_IMX8M
>  
>  config IMX8M_FEATCTRL
> -bool "i.MX8M feature controller"
> + bool "i.MX8M feature controller"
>   depends on ARCH_IMX8M
>   select FEATURE_CONTROLLER
>   default y
> + help
> +   This driver disables device tree nodes that are not applicable
> +   to Lite variants of i.MX8M SoCs.
>  
>  endmenu
> -- 
> 2.39.2
> 
> 
> 

-- 
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- |

Re: [PATCH 2/2] commands: test: add based support for bash-test style

[Sascha Hauer]

On Tue, Jul 04, 2023 at 12:58:13AM +0200, Marco Felsch wrote:
> Add [[ expression ]] support which allow pattern matching like:
> 
>   - [[ "foo1" == "foo*" ]]

It's a bit unfortunate that in barebox we require putting the pattern in
"", whereas in bash it only works without the "".

I guess there's not much we can do about this unless we turn the '['
command into a shell builtin.

>   - [[ "foo" != "bar" ]]
> 
> Signed-off-by: Marco Felsch 
> ---
>  commands/Kconfig | 13 +
>  commands/test.c  | 39 ++-
>  2 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 4d3ff631a8..615e96aa9d 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -1219,6 +1219,19 @@ config CMD_TEST
> !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, 
> -e,
> -f, -L; see 'man test' on your PC for more information.
>  
> +if CMD_TEST
> +
> +config CMD_TEST_BASH_COMP
> + tristate
> + select CONFIG_FNMATCH
> + prompt "test bash-compatibility"
> + help
> +   Enable minimal support for bash compatible tests: [[ ]] to allow
> +   pattern matching via: == and !=
> +
> +# end CMD_TEST
> +endif
> +
>  config CMD_TRUE
>   tristate
>   default y
> diff --git a/commands/test.c b/commands/test.c
> index c1b84c42ef..77e15aeb03 100644
> --- a/commands/test.c
> +++ b/commands/test.c
> @@ -11,9 +11,13 @@
>  #include 
>  #include 
>  #include 
> +#ifdef CONFIG_CMD_TEST_BASH_COMP
> +#include 
> +#endif

No need to include this conditionally.

>  
>  typedef enum {
>   OPT_EQUAL,
> + OPT_EQUAL_BASH,
>   OPT_NOT_EQUAL,
>   OPT_ARITH_EQUAL,
>   OPT_ARITH_NOT_EQUAL,
> @@ -36,6 +40,7 @@ typedef enum {
>  
>  static char *test_options[] = {
>   [OPT_EQUAL] = "=",
> + [OPT_EQUAL_BASH]= "==",
>   [OPT_NOT_EQUAL] = "!=",
>   [OPT_ARITH_EQUAL]   = "-eq",
>   [OPT_ARITH_NOT_EQUAL]   = "-ne",
> @@ -67,18 +72,37 @@ static int parse_opt(const char *opt)
>   return -1;
>  }
>  
> +static int string_comp(const char *left_op, const char *right_op, bool 
> bash_test)
> +{
> +#ifdef CONFIG_CMD_TEST_BASH_COMP
> + if (bash_test)
> + return fnmatch(right_op, left_op, 0);
> +#endif
> + return strcmp(left_op, right_op);
> +}
> +
>  static int do_test(int argc, char *argv[])
>  {
>   char **ap;
>   int left, adv, expr, last_expr, neg, last_cmp, opt, zero;
>   ulong a, b;
>   struct stat statbuf;
> + bool bash_test = false;
>  
>   if (*argv[0] == '[') {
>   argc--;
> - if (*argv[argc] != ']') {
> - printf("[: missing `]'\n");
> - return 1;
> + if (!strncmp(argv[0], "[[", 2)) {
> + if (strncmp(argv[argc], "]]", 2) != 0) {
> + printf("[[: missing `]]'\n");
> + return 1;
> + }
> + if (IS_ENABLED(CONFIG_CMD_TEST_BASH_COMP))
> + bash_test = true;

The if() is unnecessary.

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- |

Re: [PATCH 2/2] commands: test: add based support for bash-test style

[Sascha Hauer]

On Tue, Jul 04, 2023 at 10:32:22AM +0200, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 23-07-04, Ahmad Fatoum wrote:
> > On 04.07.23 00:58, Marco Felsch wrote:
> > > Add [[ expression ]] support which allow pattern matching like:
> > > 
> > >   - [[ "foo1" == "foo*" ]]
> > >   - [[ "foo" != "bar" ]]
> > > 
> > > Signed-off-by: Marco Felsch 
> > > ---
> > >  commands/Kconfig | 13 +
> > >  commands/test.c  | 39 ++-
> > >  2 files changed, 47 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/commands/Kconfig b/commands/Kconfig
> > > index 4d3ff631a8..615e96aa9d 100644
> > > --- a/commands/Kconfig
> > > +++ b/commands/Kconfig
> > > @@ -1219,6 +1219,19 @@ config CMD_TEST
> > > !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, 
> > > -e,
> > > -f, -L; see 'man test' on your PC for more information.
> > >  
> > > +if CMD_TEST
> > > +
> > > +config CMD_TEST_BASH_COMP
> > > + tristate
> > > + select CONFIG_FNMATCH
> > 
> > select FNMATCH, no CONFIG_
> 
> Right.
> 
> > Does this really need its own symbol? Can't you just use CONFIG_GLOB
> > and amend the help text?
> 
> The implementation was inspired by busybox and they use fnmatch.
> Checking glob.c I don't see any difference, therefore I would keep the
> fnmatch.

glob() is the wrong function anyway. glob() expands patterns to existing
files. You want to do pattern matching only not looking into the VFS.
fnmatch() is the right function here.

Besides, glob() uses fnmatch(), so by using glob() you would add even
more code.

> 
> I wasn't sure if every user need the bash(ism) support and sometimes
> bootloader partitions are really small due to legacy reasons. Therefore
> I went this way to let the user the choice to enable/disable the
> support.

You can't do much in barebox without globalvar support which already
uses fnmatch(), so it's likely enabled anyway.

Sascha

> > > + return fnmatch(right_op, left_op, 0);
> > > +#endif
> > 
> > Scripts that execute differently depending on a barebox config option
> > is not good user experience. I think it would be better if [[ is an
> > error if support is missing.
> 
> I was thinking about a warning which is printed once you use [[ and
> didn't enabled it before. Then I read man bash again and [[ supports
> '=' as well which equals the [ function. Therefore I dropped the
> warning.
> 
> > Also can't the #ifdef be replaced with a IS_ENABLED()
> 
> I don't think so, since the fnmatch is added conditional to keep the
> code base the same if not enabled.

Use IS_ENABLED(). The linker will discard unused functions for you. We
use this all over the 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- |

Re: [PATCH 2/2] commands: test: add based support for bash-test style

[Marco Felsch]

Hi Ahmad,

On 23-07-04, Ahmad Fatoum wrote:
> On 04.07.23 00:58, Marco Felsch wrote:
> > Add [[ expression ]] support which allow pattern matching like:
> > 
> >   - [[ "foo1" == "foo*" ]]
> >   - [[ "foo" != "bar" ]]
> > 
> > Signed-off-by: Marco Felsch 
> > ---
> >  commands/Kconfig | 13 +
> >  commands/test.c  | 39 ++-
> >  2 files changed, 47 insertions(+), 5 deletions(-)
> > 
> > diff --git a/commands/Kconfig b/commands/Kconfig
> > index 4d3ff631a8..615e96aa9d 100644
> > --- a/commands/Kconfig
> > +++ b/commands/Kconfig
> > @@ -1219,6 +1219,19 @@ config CMD_TEST
> >   !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, 
> > -e,
> >   -f, -L; see 'man test' on your PC for more information.
> >  
> > +if CMD_TEST
> > +
> > +config CMD_TEST_BASH_COMP
> > +   tristate
> > +   select CONFIG_FNMATCH
> 
> select FNMATCH, no CONFIG_

Right.

> Does this really need its own symbol? Can't you just use CONFIG_GLOB
> and amend the help text?

The implementation was inspired by busybox and they use fnmatch.
Checking glob.c I don't see any difference, therefore I would keep the
fnmatch.

I wasn't sure if every user need the bash(ism) support and sometimes
bootloader partitions are really small due to legacy reasons. Therefore
I went this way to let the user the choice to enable/disable the
support.

> > +   prompt "test bash-compatibility"
> > +   help
> > + Enable minimal support for bash compatible tests: [[ ]] to allow
> > + pattern matching via: == and !=
> > +
> > +# end CMD_TEST
> > +endif
> > +
> >  config CMD_TRUE
> > tristate
> > default y
> > diff --git a/commands/test.c b/commands/test.c
> > index c1b84c42ef..77e15aeb03 100644
> > --- a/commands/test.c
> > +++ b/commands/test.c
> > @@ -11,9 +11,13 @@
> >  #include 
> >  #include 
> >  #include 
> > +#ifdef CONFIG_CMD_TEST_BASH_COMP
> > +#include 
> > +#endif
> >  
> >  typedef enum {
> > OPT_EQUAL,
> > +   OPT_EQUAL_BASH,
> > OPT_NOT_EQUAL,
> > OPT_ARITH_EQUAL,
> > OPT_ARITH_NOT_EQUAL,
> > @@ -36,6 +40,7 @@ typedef enum {
> >  
> >  static char *test_options[] = {
> > [OPT_EQUAL] = "=",
> > +   [OPT_EQUAL_BASH]= "==",
> > [OPT_NOT_EQUAL] = "!=",
> > [OPT_ARITH_EQUAL]   = "-eq",
> > [OPT_ARITH_NOT_EQUAL]   = "-ne",
> > @@ -67,18 +72,37 @@ static int parse_opt(const char *opt)
> > return -1;
> >  }
> >  
> > +static int string_comp(const char *left_op, const char *right_op, bool 
> > bash_test)
> > +{
> > +#ifdef CONFIG_CMD_TEST_BASH_COMP
> > +   if (bash_test)
> > +   return fnmatch(right_op, left_op, 0);
> > +#endif
> 
> Scripts that execute differently depending on a barebox config option
> is not good user experience. I think it would be better if [[ is an
> error if support is missing.

I was thinking about a warning which is printed once you use [[ and
didn't enabled it before. Then I read man bash again and [[ supports
'=' as well which equals the [ function. Therefore I dropped the
warning.

> Also can't the #ifdef be replaced with a IS_ENABLED()

I don't think so, since the fnmatch is added conditional to keep the
code base the same if not enabled.

> > +   return strcmp(left_op, right_op);
> > +}
> > +
> >  static int do_test(int argc, char *argv[])
> >  {
> > char **ap;
> > int left, adv, expr, last_expr, neg, last_cmp, opt, zero;
> > ulong a, b;
> > struct stat statbuf;
> > +   bool bash_test = false;
> >  
> > if (*argv[0] == '[') {
> > argc--;
> > -   if (*argv[argc] != ']') {
> > -   printf("[: missing `]'\n");
> > -   return 1;
> > +   if (!strncmp(argv[0], "[[", 2)) {
> 
> This is equivalent to argv[0][1] == '['

I would keep the strncmp.

Regards,
  Marco

> > +   if (strncmp(argv[argc], "]]", 2) != 0) {
> > +   printf("[[: missing `]]'\n");
> > +   return 1;
> > +   }
> > +   if (IS_ENABLED(CONFIG_CMD_TEST_BASH_COMP))
> > +   bash_test = true;
> > +   } else {
> > +   if (*argv[argc] != ']') {
> > +   printf("[: missing `]'\n");
> > +   return 1;
> > +   }
> > }
> > }
> >  
> > @@ -183,10 +207,11 @@ static int do_test(int argc, char *argv[])
> > b = simple_strtol(ap[2], NULL, 0);
> > switch (parse_opt(ap[1])) {
> > case OPT_EQUAL:
> > -   expr = strcmp(ap[0], ap[2]) == 0;
> > +   case OPT_EQUAL_BASH:
> > +   expr = string_comp(ap[0], ap[2], bash_test) == 
> > 0;
> > break;
> > case OPT_NOT_EQUAL:
> > -   expr = strcmp(ap[0], ap[2]) != 0;

Re: [PATCH 1/2] ARM: i.MX8MP: Debix: refactor lowlevel setup functions

[Sascha Hauer]

On Tue, Jul 04, 2023 at 01:01:10AM +0200, Marco Felsch wrote:
> Pass the dram timing info and devicetree data as parameter instead of
> hardcode them into the function. The required information is passed by
> the ENTRY_FUNCTION after the relocation and the setup_c is done. This
> prepares the lowlevel code for adding new boards with minimal effort.
> 
> Signed-off-by: Marco Felsch 
> ---
>  arch/arm/boards/polyhex-debix/lowlevel.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/boards/polyhex-debix/lowlevel.c 
> b/arch/arm/boards/polyhex-debix/lowlevel.c
> index 1c8be39559..bb4d6f53a0 100644
> --- a/arch/arm/boards/polyhex-debix/lowlevel.c
> +++ b/arch/arm/boards/polyhex-debix/lowlevel.c
> @@ -83,7 +83,7 @@ static void power_init_board(void)
>  
>  extern struct dram_timing_info imx8mp_debix_dram_timing;
>  
> -static void start_atf(void)
> +static void start_atf(struct dram_timing_info *dram_timing)
>  {
>   /*
>* If we are in EL3 we are running for the first time and need to
> @@ -95,7 +95,7 @@ static void start_atf(void)
>  
>   power_init_board();
>  
> - imx8mp_ddr_init(_debix_dram_timing, DRAM_TYPE_LPDDR4);
> + imx8mp_ddr_init(dram_timing, DRAM_TYPE_LPDDR4);
>  
>   imx8mp_load_and_start_image_via_tfa();
>  }
> @@ -116,16 +116,17 @@ static void start_atf(void)
>   *
>   * 4. Standard barebox boot flow continues
>   */
> -static __noreturn noinline void imx8mp_debix_model_a_start(void)
> +static __noreturn noinline void
> +imx8mp_debix_start(struct dram_timing_info *dram_timing, void *dtb)
>  {
>   setup_uart();
>  
> - start_atf();
> + start_atf(dram_timing);
>  
>   /*
>* Standard entry we hit once we initialized both DDR and ATF
>*/
> - imx8mp_barebox_entry(__dtb_z_imx8mp_debix_model_a_start);
> + imx8mp_barebox_entry(dtb);
>  }
>  
>  ENTRY_FUNCTION(start_polyhex_debix, r0, r1, r2)
> @@ -135,5 +136,6 @@ ENTRY_FUNCTION(start_polyhex_debix, r0, r1, r2)
>   relocate_to_current_adr();
>   setup_c();
>  
> - imx8mp_debix_model_a_start();
> + imx8mp_debix_start(_debix_dram_timing,
> +__dtb_z_imx8mp_debix_model_a_start);
>  }
> -- 
> 2.39.2
> 
> 
> 

-- 
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- |

Re: [PATCH v2] firmware: use portable newline escape

[Sascha Hauer]

On Mon, Jul 03, 2023 at 08:55:15PM +0200, Ahmad Fatoum wrote:
> We want to print `\n' literally into the *.gen.S file, so gas turns it
> into a literal new line character. Depending on shell, the current
> scheme might not work:
> 
>   $ dash -c echo\ 'n'
>   \n
>   $ bash -c echo\ 'n'
>   \\n
> 
> Fix this by using printf instead. This fixes a cosmetic issue of non-escaped
> `\n' making it into the missing section when building without bash, e.g. on
> NixOS.
> 
> Signed-off-by: Ahmad Fatoum 
> ---
> v1 -> v2:
>   - use printf instead of echo with single quotes as the later behaves
> differently between bash and dash. Edited commit message
> appropriately.
> ---
>  firmware/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/firmware/Makefile b/firmware/Makefile
> index d7f35853a404..5853950dfe4e 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -62,7 +62,7 @@ filechk_fwbin = { \
>   echo "\#ifdef __PBL__"  ;\
>   echo ".section .missing_fw,\"a\""   ;\
>   echo "_fwname_$(FWSTR):";\
> - echo ".ascii \"firmware/$(FWNAME)n\""   ;\
> + printf '.ascii "%s"\n' 'firmware/$(FWNAME)\n'   ;\
>   echo "\#endif"  ;\
>  }
>  
> -- 
> 2.39.2
> 
> 
> 

-- 
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- |

Re: v2023.07.0

[Sascha Hauer]

On Mon, Jul 03, 2023 at 04:44:02PM +0200, Ahmad Fatoum wrote:
> Hi,
> 
> On 03.07.23 16:08, Sascha Hauer wrote:
> >   firmware: reference pointer alignment defined in 
> > 
> 
> There's a possible issue here for platforms that use firmware
> built into barebox (e.g. Rockchip, i.MX8M). See:
> 
> https://lore.barebox.org/barebox/20230703142122.1522533-1-a.fat...@pengutronix.de/T/#u

D'oh. Where was that lost?

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- |

Re: [PATCH] firmware: fix alignment of firmware images

[Sascha Hauer]

On Mon, Jul 03, 2023 at 04:21:22PM +0200, Ahmad Fatoum wrote:
> ASM_LGPTR is a preprocessor symbol, not a Make variable, so expanding it
> in the Makefile results in .p2align without an argument, which disables
> alignment requirement. Fix this to restore the sizeof(void *) alignment
> we had before.
> 
> Fixes: a59e2740b4a7 ("firmware: reference pointer alignment defined in 
> ")
> Reported-by: Sascha Hauer 
> Signed-off-by: Ahmad Fatoum 
> ---
>  firmware/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 8d3bfb0752ff..d7f35853a404 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -49,7 +49,7 @@ filechk_fwbin = { \
>   echo "\#include ";\
>   echo ".section .note.GNU-stack,\"\",%progbits"  ;\
>   echo ".section $2,\"$3\""   ;\
> - echo ".p2align $(ASM_LGPTR)";\
> + echo ".p2align ASM_LGPTR"   ;\
>   echo ".global _fw_$(FWSTR)_start"   ;\
>   echo "_fw_$(FWSTR)_start:"  ;\
>   echo "\#if $(FWNAME_EXISTS)";\
> @@ -68,7 +68,7 @@ filechk_fwbin = { \
>  
>  __fwbin_sha = { \
>   echo ".section .rodata.$(FWSTR).sha";\
> - echo ".p2align $(ASM_LGPTR)";\
> + echo ".p2align ASM_LGPTR"   ;\
>   echo ".global _fw_$(FWSTR)_sha_start"   ;\
>   echo "_fw_$(FWSTR)_sha_start:"  ;\
>   echo ".incbin \"$(fwobjdir)/$(FWNAME).sha.bin\"";\
> -- 
> 2.39.2
> 
> 

-- 
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- |

Re: [PATCH 1/2] ARM: i.MX8MP: Debix: refactor lowlevel setup functions

[Ahmad Fatoum]

On 04.07.23 01:01, Marco Felsch wrote:
> Pass the dram timing info and devicetree data as parameter instead of
> hardcode them into the function. The required information is passed by
> the ENTRY_FUNCTION after the relocation and the setup_c is done. This
> prepares the lowlevel code for adding new boards with minimal effort.
> 
> Signed-off-by: Marco Felsch 

Reviewed-by: Ahmad Fatoum 

> ---
>  arch/arm/boards/polyhex-debix/lowlevel.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boards/polyhex-debix/lowlevel.c 
> b/arch/arm/boards/polyhex-debix/lowlevel.c
> index 1c8be39559..bb4d6f53a0 100644
> --- a/arch/arm/boards/polyhex-debix/lowlevel.c
> +++ b/arch/arm/boards/polyhex-debix/lowlevel.c
> @@ -83,7 +83,7 @@ static void power_init_board(void)
>  
>  extern struct dram_timing_info imx8mp_debix_dram_timing;
>  
> -static void start_atf(void)
> +static void start_atf(struct dram_timing_info *dram_timing)
>  {
>   /*
>* If we are in EL3 we are running for the first time and need to
> @@ -95,7 +95,7 @@ static void start_atf(void)
>  
>   power_init_board();
>  
> - imx8mp_ddr_init(_debix_dram_timing, DRAM_TYPE_LPDDR4);
> + imx8mp_ddr_init(dram_timing, DRAM_TYPE_LPDDR4);
>  
>   imx8mp_load_and_start_image_via_tfa();
>  }
> @@ -116,16 +116,17 @@ static void start_atf(void)
>   *
>   * 4. Standard barebox boot flow continues
>   */
> -static __noreturn noinline void imx8mp_debix_model_a_start(void)
> +static __noreturn noinline void
> +imx8mp_debix_start(struct dram_timing_info *dram_timing, void *dtb)
>  {
>   setup_uart();
>  
> - start_atf();
> + start_atf(dram_timing);
>  
>   /*
>* Standard entry we hit once we initialized both DDR and ATF
>*/
> - imx8mp_barebox_entry(__dtb_z_imx8mp_debix_model_a_start);
> + imx8mp_barebox_entry(dtb);
>  }
>  
>  ENTRY_FUNCTION(start_polyhex_debix, r0, r1, r2)
> @@ -135,5 +136,6 @@ ENTRY_FUNCTION(start_polyhex_debix, r0, r1, r2)
>   relocate_to_current_adr();
>   setup_c();
>  
> - imx8mp_debix_model_a_start();
> + imx8mp_debix_start(_debix_dram_timing,
> +__dtb_z_imx8mp_debix_model_a_start);
>  }

-- 
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- |

Re: [PATCH 2/2] commands: test: add based support for bash-test style

[Ahmad Fatoum]

On 04.07.23 00:58, Marco Felsch wrote:
> Add [[ expression ]] support which allow pattern matching like:
> 
>   - [[ "foo1" == "foo*" ]]
>   - [[ "foo" != "bar" ]]
> 
> Signed-off-by: Marco Felsch 
> ---
>  commands/Kconfig | 13 +
>  commands/test.c  | 39 ++-
>  2 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 4d3ff631a8..615e96aa9d 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -1219,6 +1219,19 @@ config CMD_TEST
> !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, 
> -e,
> -f, -L; see 'man test' on your PC for more information.
>  
> +if CMD_TEST
> +
> +config CMD_TEST_BASH_COMP
> + tristate
> + select CONFIG_FNMATCH

select FNMATCH, no CONFIG_
Does this really need its own symbol? Can't you just use CONFIG_GLOB
and amend the help text?

> + prompt "test bash-compatibility"
> + help
> +   Enable minimal support for bash compatible tests: [[ ]] to allow
> +   pattern matching via: == and !=
> +
> +# end CMD_TEST
> +endif
> +
>  config CMD_TRUE
>   tristate
>   default y
> diff --git a/commands/test.c b/commands/test.c
> index c1b84c42ef..77e15aeb03 100644
> --- a/commands/test.c
> +++ b/commands/test.c
> @@ -11,9 +11,13 @@
>  #include 
>  #include 
>  #include 
> +#ifdef CONFIG_CMD_TEST_BASH_COMP
> +#include 
> +#endif
>  
>  typedef enum {
>   OPT_EQUAL,
> + OPT_EQUAL_BASH,
>   OPT_NOT_EQUAL,
>   OPT_ARITH_EQUAL,
>   OPT_ARITH_NOT_EQUAL,
> @@ -36,6 +40,7 @@ typedef enum {
>  
>  static char *test_options[] = {
>   [OPT_EQUAL] = "=",
> + [OPT_EQUAL_BASH]= "==",
>   [OPT_NOT_EQUAL] = "!=",
>   [OPT_ARITH_EQUAL]   = "-eq",
>   [OPT_ARITH_NOT_EQUAL]   = "-ne",
> @@ -67,18 +72,37 @@ static int parse_opt(const char *opt)
>   return -1;
>  }
>  
> +static int string_comp(const char *left_op, const char *right_op, bool 
> bash_test)
> +{
> +#ifdef CONFIG_CMD_TEST_BASH_COMP
> + if (bash_test)
> + return fnmatch(right_op, left_op, 0);
> +#endif

Scripts that execute differently depending on a barebox config option
is not good user experience. I think it would be better if [[ is an
error if support is missing.

Also can't the #ifdef be replaced with a IS_ENABLED()

> + return strcmp(left_op, right_op);
> +}
> +
>  static int do_test(int argc, char *argv[])
>  {
>   char **ap;
>   int left, adv, expr, last_expr, neg, last_cmp, opt, zero;
>   ulong a, b;
>   struct stat statbuf;
> + bool bash_test = false;
>  
>   if (*argv[0] == '[') {
>   argc--;
> - if (*argv[argc] != ']') {
> - printf("[: missing `]'\n");
> - return 1;
> + if (!strncmp(argv[0], "[[", 2)) {

This is equivalent to argv[0][1] == '['

> + if (strncmp(argv[argc], "]]", 2) != 0) {
> + printf("[[: missing `]]'\n");
> + return 1;
> + }
> + if (IS_ENABLED(CONFIG_CMD_TEST_BASH_COMP))
> + bash_test = true;
> + } else {
> + if (*argv[argc] != ']') {
> + printf("[: missing `]'\n");
> + return 1;
> + }
>   }
>   }
>  
> @@ -183,10 +207,11 @@ static int do_test(int argc, char *argv[])
>   b = simple_strtol(ap[2], NULL, 0);
>   switch (parse_opt(ap[1])) {
>   case OPT_EQUAL:
> - expr = strcmp(ap[0], ap[2]) == 0;
> + case OPT_EQUAL_BASH:
> + expr = string_comp(ap[0], ap[2], bash_test) == 
> 0;
>   break;
>   case OPT_NOT_EQUAL:
> - expr = strcmp(ap[0], ap[2]) != 0;
> + expr = string_comp(ap[0], ap[2], bash_test) != 
> 0;
>   break;
>   case OPT_ARITH_EQUAL:
>   expr = a == b;
> @@ -233,7 +258,11 @@ static int do_test(int argc, char *argv[])
>   return expr;
>  }
>  
> +#ifdef CONFIG_CMD_TEST_BASH_COMP
> +static const char * const test_aliases[] = { "[", "[[", NULL};
> +#else
>  static const char * const test_aliases[] = { "[", NULL};
> +#endif
>  
>  BAREBOX_CMD_HELP_START(test)
>  BAREBOX_CMD_HELP_TEXT("Options:")

-- 
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- |