date:20210813

Hi all,

On 04/08/21 8:08 pm, Aswath Govindraju wrote:
> Add support for enumerating MMC card in a given mode using mmc rescan and
> mmc dev commands. The speed mode is provided as the last argument in these
> commands and is indicated using the index from enum bus_mode in
> include/mmc.h. A speed mode can be set only if it has already been enabled
> in the device tree.
> 
> Signed-off-by: Aswath Govindraju 
> ---

Posted respin(v4) for this patch seperating the documentation changes.

Thanks,
Aswath

> 
> Changes since v2:
> - made init_mmc_device as wrapper function around __init_mmc_device
> - rebased on top of latest master
> - used dectoul() instead of simple_strtoul() whereever possible
> 
> Changes since v1:
> - Removed #ifdef around the definition of user_speed_mode field
> - Replaced #if with if (IS_ENABLED())
> - Removed repeated MMC_CAP(MMC_LEGACY)
> - initialized user_speed_mode to MMC_MODES_SPEED in mmc_init_device
>   and mmc_dp_preinit
> - corrected curr_device to dev and included part check, the case when
>   mode is provided in the argument
> 
>  cmd/Kconfig  | 10 
>  cmd/mmc.c| 52 +---
>  doc/usage/mmc.rst| 49 +++--
>  drivers/mmc/mmc-uclass.c |  5 +++-
>  drivers/mmc/mmc.c| 22 -
>  include/mmc.h|  2 ++
>  6 files changed, 128 insertions(+), 12 deletions(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index ffef3cc76ca4..3a857b3f6e2e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2389,4 +2389,14 @@ config CMD_UBIFS
>   help
> UBIFS is a file system for flash devices which works on top of UBI.
>  
> +config MMC_SPEED_MODE_SET
> + bool "set speed mode using mmc command"
> + depends on CMD_MMC
> + default n
> + help
> +   Enable setting speed mode using mmc rescan and mmc dev commands.
> +   The speed mode is provided as the last argument in these commands
> +   and is indicated using the index from enum bus_mode in
> +   include/mmc.h. A speed mode can be set only if it has already
> +   been enabled in the device tree.
>  endmenu
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index c67ad7624227..f1e30d0cf64b 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -120,7 +120,9 @@ static void print_mmcinfo(struct mmc *mmc)
>   }
>   }
>  }
> -static struct mmc *init_mmc_device(int dev, bool force_init)
> +
> +static struct mmc *__init_mmc_device(int dev, bool force_init,
> +  enum bus_mode speed_mode)
>  {
>   struct mmc *mmc;
>   mmc = find_mmc_device(dev);
> @@ -134,6 +136,10 @@ static struct mmc *init_mmc_device(int dev, bool 
> force_init)
>  
>   if (force_init)
>   mmc->has_init = 0;
> +
> + if (IS_ENABLED(CONFIG_MMC_SPEED_MODE_SET))
> + mmc->user_speed_mode = speed_mode;
> +
>   if (mmc_init(mmc))
>   return NULL;
>  
> @@ -145,6 +151,11 @@ static struct mmc *init_mmc_device(int dev, bool 
> force_init)
>   return mmc;
>  }
>  
> +static struct mmc *init_mmc_device(int dev, bool force_init)
> +{
> + return __init_mmc_device(dev, force_init, MMC_MODES_END);
> +}
> +
>  static int do_mmcinfo(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
>  {
> @@ -482,8 +493,17 @@ static int do_mmc_rescan(struct cmd_tbl *cmdtp, int flag,
>int argc, char *const argv[])
>  {
>   struct mmc *mmc;
> + enum bus_mode speed_mode = MMC_MODES_END;
> +
> + if (argc == 1) {
> + mmc = init_mmc_device(curr_device, true);
> + } else if (argc == 2) {
> + speed_mode = (int)dectoul(argv[1], NULL);
> + mmc = __init_mmc_device(curr_device, true, speed_mode);
> + } else {
> + return CMD_RET_USAGE;
> + }
>  
> - mmc = init_mmc_device(curr_device, true);
>   if (!mmc)
>   return CMD_RET_FAILURE;
>  
> @@ -515,11 +535,14 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag,
>  {
>   int dev, part = 0, ret;
>   struct mmc *mmc;
> + enum bus_mode speed_mode = MMC_MODES_END;
>  
>   if (argc == 1) {
>   dev = curr_device;
> + mmc = init_mmc_device(dev, true);
>   } else if (argc == 2) {
> - dev = dectoul(argv[1], NULL);
> + dev = (int)dectoul(argv[1], NULL);
> + mmc = init_mmc_device(dev, true);
>   } else if (argc == 3) {
>   dev = (int)dectoul(argv[1], NULL);
>   part = (int)dectoul(argv[2], NULL);
> @@ -528,11 +551,21 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag,
>  PART_ACCESS_MASK);
>   return CMD_RET_FAILURE;
>   }
> + mmc = init_mmc_device(dev, true);
> + } else if (argc == 4) {
> + dev = (int)dectoul(argv[1], NULL);
> + part = (int)dectoul(argv[2], NULL);

[PATCH v4 2/2] doc: usage: mmc: Document usage of speed mode in "mmc dev" and "mmc rescan"

Add documentation on the usage of "mmc dev" and "mmc rescan" commands to
set user defined speed modes.

Signed-off-by: Aswath Govindraju 
---
 doc/usage/mmc.rst | 49 +--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/doc/usage/mmc.rst b/doc/usage/mmc.rst
index f20efe3d7063..d15b151884fd 100644
--- a/doc/usage/mmc.rst
+++ b/doc/usage/mmc.rst
@@ -12,9 +12,9 @@ Synopsis
 mmc read addr blk# cnt
 mmc write addr blk# cnt
 mmc erase blk# cnt
-mmc rescan
+mmc rescan [mode]
 mmc part
-mmc dev [dev] [part]
+mmc dev [dev] [part] [mode]
 mmc list
 mmc wp
 mmc bootbus
@@ -49,6 +49,27 @@ The 'mmc erase' command erases *cnt* blocks on the MMC 
device starting at block
 
 The 'mmc rescan' command scans the available MMC device.
 
+   mode
+   speed mode to set.
+   CONFIG_MMC_SPEED_MODE_SET should be enabled. The required speed mode is
+   passed as the index from the following list.
+
+   0   - MMC_LEGACY
+   1   - MMC_HS
+   2   - SD_HS
+   3   - MMC_HS_52
+   4   - MMC_DDR_52
+   5   - UHS_SDR12
+   6   - UHS_SDR25
+   7   - UHS_SDR50
+   8   - UHS_DDR50
+   9   - UHS_SDR104
+   10  - MMC_HS_200
+   11  - MMC_HS_400
+   12  - MMC_HS_400_ES
+
+   A speed mode can be set only if it has already been enabled in the 
device tree
+
 The 'mmc part' command displays the list available partition on current mmc 
device.
 
 The 'mmc dev' command shows or set current mmc device.
@@ -58,6 +79,27 @@ The 'mmc dev' command shows or set current mmc device.
 part
 partition number to change
 
+   mode
+   speed mode to set.
+   CONFIG_MMC_SPEED_MODE_SET should be enabled. The required speed mode is
+   passed as the index from the following list.
+
+   0   - MMC_LEGACY
+   1   - MMC_HS
+   2   - SD_HS
+   3   - MMC_HS_52
+   4   - MMC_DDR_52
+   5   - UHS_SDR12
+   6   - UHS_SDR25
+   7   - UHS_SDR50
+   8   - UHS_DDR50
+   9   - UHS_SDR104
+   10  - MMC_HS_200
+   11  - MMC_HS_400
+   12  - MMC_HS_400_ES
+
+   A speed mode can be set only if it has already been enabled in the 
device tree
+
 The 'mmc list' command displays the list available devices.
 
 The 'mmc wp' command enables "power on write protect" function for boot 
partitions.
@@ -194,6 +236,9 @@ The current device can be shown or set via 'mmc dev' 
command:
 => mmc dev 2 0
 switch to partitions #0, OK
 mmc2 is current device
+=> mmc dev 0 1 4
+switch to partitions #1, OK
+mmc0(part 1) is current device
 
 The list of available devices can be shown via 'mmc list' command:
 ::
-- 
2.17.1

[PATCH v4 1/2] mmc: Add support for enumerating MMC card in a given mode using mmc command

Add support for enumerating MMC card in a given mode using mmc rescan and
mmc dev commands. The speed mode is provided as the last argument in these
commands and is indicated using the index from enum bus_mode in
include/mmc.h. A speed mode can be set only if it has already been enabled
in the device tree.

Signed-off-by: Aswath Govindraju 
---
 cmd/Kconfig  | 10 
 cmd/mmc.c| 52 +---
 drivers/mmc/mmc-uclass.c |  5 +++-
 drivers/mmc/mmc.c| 22 -
 include/mmc.h|  2 ++
 5 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index ffef3cc76ca4..3a857b3f6e2e 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2389,4 +2389,14 @@ config CMD_UBIFS
help
  UBIFS is a file system for flash devices which works on top of UBI.
 
+config MMC_SPEED_MODE_SET
+   bool "set speed mode using mmc command"
+   depends on CMD_MMC
+   default n
+   help
+ Enable setting speed mode using mmc rescan and mmc dev commands.
+ The speed mode is provided as the last argument in these commands
+ and is indicated using the index from enum bus_mode in
+ include/mmc.h. A speed mode can be set only if it has already
+ been enabled in the device tree.
 endmenu
diff --git a/cmd/mmc.c b/cmd/mmc.c
index c67ad7624227..f1e30d0cf64b 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -120,7 +120,9 @@ static void print_mmcinfo(struct mmc *mmc)
}
}
 }
-static struct mmc *init_mmc_device(int dev, bool force_init)
+
+static struct mmc *__init_mmc_device(int dev, bool force_init,
+enum bus_mode speed_mode)
 {
struct mmc *mmc;
mmc = find_mmc_device(dev);
@@ -134,6 +136,10 @@ static struct mmc *init_mmc_device(int dev, bool 
force_init)
 
if (force_init)
mmc->has_init = 0;
+
+   if (IS_ENABLED(CONFIG_MMC_SPEED_MODE_SET))
+   mmc->user_speed_mode = speed_mode;
+
if (mmc_init(mmc))
return NULL;
 
@@ -145,6 +151,11 @@ static struct mmc *init_mmc_device(int dev, bool 
force_init)
return mmc;
 }
 
+static struct mmc *init_mmc_device(int dev, bool force_init)
+{
+   return __init_mmc_device(dev, force_init, MMC_MODES_END);
+}
+
 static int do_mmcinfo(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
@@ -482,8 +493,17 @@ static int do_mmc_rescan(struct cmd_tbl *cmdtp, int flag,
 int argc, char *const argv[])
 {
struct mmc *mmc;
+   enum bus_mode speed_mode = MMC_MODES_END;
+
+   if (argc == 1) {
+   mmc = init_mmc_device(curr_device, true);
+   } else if (argc == 2) {
+   speed_mode = (int)dectoul(argv[1], NULL);
+   mmc = __init_mmc_device(curr_device, true, speed_mode);
+   } else {
+   return CMD_RET_USAGE;
+   }
 
-   mmc = init_mmc_device(curr_device, true);
if (!mmc)
return CMD_RET_FAILURE;
 
@@ -515,11 +535,14 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag,
 {
int dev, part = 0, ret;
struct mmc *mmc;
+   enum bus_mode speed_mode = MMC_MODES_END;
 
if (argc == 1) {
dev = curr_device;
+   mmc = init_mmc_device(dev, true);
} else if (argc == 2) {
-   dev = dectoul(argv[1], NULL);
+   dev = (int)dectoul(argv[1], NULL);
+   mmc = init_mmc_device(dev, true);
} else if (argc == 3) {
dev = (int)dectoul(argv[1], NULL);
part = (int)dectoul(argv[2], NULL);
@@ -528,11 +551,21 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag,
   PART_ACCESS_MASK);
return CMD_RET_FAILURE;
}
+   mmc = init_mmc_device(dev, true);
+   } else if (argc == 4) {
+   dev = (int)dectoul(argv[1], NULL);
+   part = (int)dectoul(argv[2], NULL);
+   if (part > PART_ACCESS_MASK) {
+   printf("#part_num shouldn't be larger than %d\n",
+  PART_ACCESS_MASK);
+   return CMD_RET_FAILURE;
+   }
+   speed_mode = (int)dectoul(argv[3], NULL);
+   mmc = __init_mmc_device(dev, true, speed_mode);
} else {
return CMD_RET_USAGE;
}
 
-   mmc = init_mmc_device(dev, true);
if (!mmc)
return CMD_RET_FAILURE;
 
@@ -983,9 +1016,9 @@ static struct cmd_tbl cmd_mmc[] = {
 #if CONFIG_IS_ENABLED(CMD_MMC_SWRITE)
U_BOOT_CMD_MKENT(swrite, 3, 0, do_mmc_sparse_write, "", ""),
 #endif
-   U_BOOT_CMD_MKENT(rescan, 1, 1, do_mmc_rescan, "", ""),
+   U_BOOT_CMD_MKENT(rescan, 2, 1, do_mmc_rescan, "", ""),
U_BOOT_CMD_MKENT(part, 1, 1, do_mmc_part, "", ""),
-   U_BOOT_

[PATCH v4 0/2] mmc: Add support for enumrating MMC card in user specified mode

The following series of patches,
- add support enumerating MMC card in user specified speed mode
- add documentation for the usage of above feature

changes since v3:
- split the patch in two, seperating the documentation changes

Aswath Govindraju (2):
  mmc: Add support for enumerating MMC card in a given mode using mmc
command
  doc: usage: mmc: Document usage of speed mode in "mmc dev" and "mmc
rescan"

 cmd/Kconfig  | 10 
 cmd/mmc.c| 52 +---
 doc/usage/mmc.rst| 49 +++--
 drivers/mmc/mmc-uclass.c |  5 +++-
 drivers/mmc/mmc.c| 22 -
 include/mmc.h|  2 ++
 6 files changed, 128 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH v2 2/2] tools: Handle PAGER containing arguments

2021-08-13 Thread Paul Barker

When printing full help output from a tool, we should be able to handle
a PAGER variable which includes arguments, e.g. PAGER='less -F'.

Signed-off-by: Paul Barker 
---
 tools/patman/tools.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index 96882264a2..92e3240470 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -5,6 +5,7 @@
 
 import glob
 import os
+import shlex
 import shutil
 import struct
 import sys
@@ -588,9 +589,9 @@ def PrintFullHelp(fname):
 Args:
 fname: Path to a file containing the full help message
 """
-pager = os.getenv('PAGER')
+pager = shlex.split(os.getenv('PAGER'))
 if not pager:
-pager = shutil.which('less')
+pager = [shutil.which('less')]
 if not pager:
-pager = 'more'
-command.Run(pager, fname)
+pager = ['more']
+command.Run(*pager, fname)
-- 
2.31.1

[PATCH v2 1/2] tools: Refactor full help printing

2021-08-13 Thread Paul Barker

Collect the code for printing the full help message of patman, buildman
and binman into a single function in patman.tools.

Signed-off-by: Paul Barker 
---
 tools/binman/control.py   |  9 +++--
 tools/buildman/control.py | 10 --
 tools/patman/main.py  | 12 
 tools/patman/tools.py | 13 +
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index dcba02ff7f..0dbcbc28e9 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -565,12 +565,9 @@ def Binman(args):
 global state
 
 if args.full_help:
-pager = os.getenv('PAGER')
-if not pager:
-pager = 'more'
-fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
-'README.rst')
-command.Run(pager, fname)
+tools.PrintFullHelp(
+os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), 
'README.rst')
+)
 return 0
 
 # Put these here so that we can import this module without libfdt
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index a98d1b4c06..fd9664c85d 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -16,6 +16,7 @@ from patman import command
 from patman import gitutil
 from patman import patchstream
 from patman import terminal
+from patman import tools
 from patman.terminal import Print
 
 def GetPlural(count):
@@ -133,12 +134,9 @@ def DoBuildman(options, args, toolchains=None, 
make_func=None, boards=None,
 global builder
 
 if options.full_help:
-pager = os.getenv('PAGER')
-if not pager:
-pager = 'more'
-fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
- 'README')
-command.Run(pager, fname)
+tools.PrintFullHelp(
+os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), 
'README')
+)
 return 0
 
 gitutil.Setup()
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 04e37a5931..e5be28e331 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -28,6 +28,7 @@ from patman import settings
 from patman import terminal
 from patman import test_util
 from patman import test_checkpatch
+from patman import tools
 
 epilog = '''Create patches from commits in a branch, check them and email them
 as specified by tags you place in the commits. Use -n to do a dry run first.'''
@@ -170,14 +171,9 @@ elif args.cmd == 'send':
 fd.close()
 
 elif args.full_help:
-pager = os.getenv('PAGER')
-if not pager:
-pager = shutil.which('less')
-if not pager:
-pager = 'more'
-fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
- 'README')
-command.Run(pager, fname)
+tools.PrintFullHelp(
+os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), 
'README')
+)
 
 else:
 # If we are not processing tags, no need to warning about bad ones
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index 877e37cd8d..96882264a2 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -581,3 +581,16 @@ def ToHexSize(val):
 hex value of size, or 'None' if the value is None
 """
 return 'None' if val is None else '%#x' % len(val)
+
+def PrintFullHelp(fname):
+"""Print the full help message for a tool using an appropriate pager.
+
+Args:
+fname: Path to a file containing the full help message
+"""
+pager = os.getenv('PAGER')
+if not pager:
+pager = shutil.which('less')
+if not pager:
+pager = 'more'
+command.Run(pager, fname)
-- 
2.31.1

[PATCH v2 0/2] Refactor and improve full help output from tools

2021-08-13 Thread Paul Barker

Sorry this took so long to get back to! Got there in the end though :)

Changes from v1:
  * Collected the full help printing code from patman, buildman & binman into a
single function so that when support for PAGER containing arguments is added
it applies to all the relevant tools.

Paul Barker (2):
  tools: Refactor full help printing
  tools: Handle PAGER containing arguments

 tools/binman/control.py   |  9 +++--
 tools/buildman/control.py | 10 --
 tools/patman/main.py  | 12 
 tools/patman/tools.py | 14 ++
 4 files changed, 25 insertions(+), 20 deletions(-)

-- 
2.31.1

Re: [PATCH v1] imx: mkimage_fit_atf: replace @ with hyphen

2021-08-13 Thread Heiko Thiery

Hi Fabio,

Am Fr., 13. Aug. 2021 um 15:03 Uhr schrieb Fabio Estevam :
>
> Hi Heiko,
>
> On Fri, Aug 13, 2021 at 9:53 AM Heiko Thiery  wrote:
>
> > Maybe I'm wrong or I do not fully understand the limitation you're
> > talking about, but as far as I understand the output (flash.bin) from
> > the ronetix board [1] was generated using binman and includes all
> > necessary parts. Is this right?
>
> Building the imx8mq_cm_defconfig generates flash.bin + u-boot.itb.
>
> If you only flash flash.bin, then you don't have a bootable system as
> flash.bin contains only the SPL part.
>
> Prior to using binman, we could boot the i.MX8M boards by flashing
> only flash.bin.
>
> I would like to preserver the pre-U-Boot 2021.07, where only flash.bin
> was needed.

I had taken the binman configuration from imx8mq_cm_defconfig and
adapted it for my board (kontron-pitx-imx8m). I had copied the
flash.bin image to my SD card and am of the opinion that this had
started afterwards with the current u-boot. Unfortunately I can test
it only next week again, because I currently have no device at hand.

I must confess that I do not yet fully understand how binman works.

I'll get back to you next week.

-- 
Heiko

[PATCH] gpio: mxc_gpio: Fix i.MX8M GPIO output status read

2021-08-13 Thread Harm Berntsen

Currently the driver gets value from PSR register, but this register is
only for input mode. For output mode, it always returns 0, not the value
we set for output.

This patch changes to use DR register, which returns the DR value for
output mode, and PSR value for input mode.

This patch is based on code from Ye Li 

Signed-off-by: Robert Krikke 
Signed-off-by: Harm Berntsen 
CC: Ye Li 
CC: Stefano Babic 
---

 drivers/gpio/mxc_gpio.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
index 06e6b2279f..c30246399c 100644
--- a/drivers/gpio/mxc_gpio.c
+++ b/drivers/gpio/mxc_gpio.c
@@ -36,6 +36,17 @@ struct mxc_bank_info {
struct gpio_regs *regs;
 };
 
+static int gpio_bank_get_value(struct gpio_regs *regs, int offset)
+{
+   u32 gpio_val;
+
+   if (IS_ENABLED(CONFIG_ARCH_IMX8M))
+   gpio_val = readl(®s->gpio_dr);
+   else
+   gpio_val = readl(®s->gpio_psr);
+   return (gpio_val >> offset) & 0x01;
+}
+
 #if !CONFIG_IS_ENABLED(DM_GPIO)
 #define GPIO_TO_PORT(n)((n) / 32)
 
@@ -125,18 +136,13 @@ int gpio_get_value(unsigned gpio)
 {
unsigned int port = GPIO_TO_PORT(gpio);
struct gpio_regs *regs;
-   u32 val;
-
if (port >= ARRAY_SIZE(gpio_ports))
return -1;
 
gpio &= 0x1f;
 
regs = (struct gpio_regs *)gpio_ports[port];
-
-   val = (readl(®s->gpio_psr) >> gpio) & 0x01;
-
-   return val;
+   return gpio_bank_get_value(regs, gpio);
 }
 
 int gpio_request(unsigned gpio, const char *label)
@@ -211,7 +217,7 @@ static void mxc_gpio_bank_set_value(struct gpio_regs *regs, 
int offset,
 
 static int mxc_gpio_bank_get_value(struct gpio_regs *regs, int offset)
 {
-   return (readl(®s->gpio_psr) >> offset) & 0x01;
+   return gpio_bank_get_value(regs, offset);
 }
 
 /* set GPIO pin 'gpio' as an input */
-- 
2.32.0

Re: [PATCH v3 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

On Friday 13 August 2021 01:39:37 Luka Kovacic wrote:
> Add the loadaddr U-Boot environment variable, as this is available in
> the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
> 
> Signed-off-by: Luka Kovacic 
> Cc: Luka Perkov 
> Cc: Robert Marko 
> ---
>  include/configs/mvebu_armada-37xx.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/mvebu_armada-37xx.h 
> b/include/configs/mvebu_armada-37xx.h
> index 8e8bcfa018..6901680e32 100644
> --- a/include/configs/mvebu_armada-37xx.h
> +++ b/include/configs/mvebu_armada-37xx.h
> @@ -110,6 +110,7 @@
>  
>  /* fdt_addr and kernel_addr are needed for existing distribution boot 
> scripts */
>  #define CONFIG_EXTRA_ENV_SETTINGS\
> + "loadaddr=0x600\0"  \

Now I see where is the issue...

In file include/env_default.h is:

#ifdef  CONFIG_LOADADDR
"loadaddr=" __stringify(CONFIG_LOADADDR)"\0"
#endif

So default value for loadaddr= is set only when CONFIG_LOADADDR is
defined.

But lot of cmd load commands are using config option
CONFIG_SYS_LOAD_ADDR as a default value for load address.

And also for espressobin we set correct value into CONFIG_SYS_LOAD_ADDR.

I'm looking at the u-boot code and CONFIG_LOADADDR is used only by
cmd/qfw.c and include/env_default.h. And on all other places (in cmd
load commands) is used CONFIG_SYS_LOAD_ADDR.

So for me it looks like that cmd/qfw.c and include/env_default.h should
be fixed to use CONFIG_SYS_LOAD_ADDR instead of CONFIG_LOADADDR.

>   "scriptaddr=0x6d0\0"\
>   "pxefile_addr_r=0x6e0\0"\
>   "fdt_addr=0x6f0\0"  \
> -- 
> 2.31.1
>

Re: [PATCH v1] imx: mkimage_fit_atf: replace @ with hyphen

Hi Heiko,

On Fri, Aug 13, 2021 at 9:53 AM Heiko Thiery  wrote:

> Maybe I'm wrong or I do not fully understand the limitation you're
> talking about, but as far as I understand the output (flash.bin) from
> the ronetix board [1] was generated using binman and includes all
> necessary parts. Is this right?

Building the imx8mq_cm_defconfig generates flash.bin + u-boot.itb.

If you only flash flash.bin, then you don't have a bootable system as
flash.bin contains only the SPL part.

Prior to using binman, we could boot the i.MX8M boards by flashing
only flash.bin.

I would like to preserver the pre-U-Boot 2021.07, where only flash.bin
was needed.

>
> [1] 
> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/imx8mq-cm-u-boot.dtsi
>
>
> --
> Heiko

Re: [PATCH v3 2/9] arm: dts: imx8mp: Add common u-boot dtsi

Hi Heiko,

On Sat, Jul 10, 2021 at 9:23 AM Heiko Schocher  wrote:

> > Doesn't this change from having a single flash.bin encompasing the SPL
> > and U-Boot proper to having split files? I noticed that happened with
> > imx8mm_evk for example when it switched to binman.
>
> Yes, but you can easy generate there a single image again.

Yes, that would be preferable

Any suggestions on how we can have a single bootable image again?

Thanks,

Fabio Estevam

Re: [PATCH v1] imx: mkimage_fit_atf: replace @ with hyphen

2021-08-13 Thread Heiko Thiery

Hi,

Am Fr., 13. Aug. 2021 um 14:34 Uhr schrieb Fabio Estevam :
>
> Hi Tim,
>
> [Adding Marek]
>
> On Tue, Jul 27, 2021 at 6:53 PM Tim Harvey  wrote:
>
> > With respect to moving to binman, I'm all for it but I'm wondering why
> > other boards have elected to move from a monolithic flash.bin
> > including the SPL and u-boot.itb pre-binman to post-binman where
> > flash.bin is just the SPL. I'm not sure why users should be hit with a
> > change like that... why are people not having binman produce a
> > flash.bin that was equivalent to pre-binman?
>
> Yes, fully agree.
>
> Peng,
>
> After the adoption of binman, both flash.bin and u-boot.itb need to be
> flashed to the boot media.
>
> This breaks user experience, distro integration, etc, which is not good.
>
> Can we have binman generate a single bootable flash.bin binary (which
> contains flash.bin + u-boot.itb) again
> to keep compatibility and avoid breakage?

Maybe I'm wrong or I do not fully understand the limitation you're
talking about, but as far as I understand the output (flash.bin) from
the ronetix board [1] was generated using binman and includes all
necessary parts. Is this right?

[1] 
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/imx8mq-cm-u-boot.dtsi


-- 
Heiko

Re: Please pull u-boot-x86

2021-08-13 Thread Tom Rini

On Fri, Aug 13, 2021 at 01:11:56PM +0800, Bin Meng wrote:

> Hi Tom,
> 
> This PR includes the following x86 changes for v2021.10:
> 
> - Enable SeaBIOS support for Crown Bay
> - Update SeaBIOS build instructions in the x86 doc
> - Enable CONFIG_SPI_FLASH_SMART_HWCAPS for Crown Bay
> 
> Azure results: PASS
> https://dev.azure.com/bmeng/GitHub/_build/results?buildId=419&view=results
> 
> The following changes since commit a25277122dad99837b78cd3b3ae6b8214df88c26:
> 
>   Merge https://source.denx.de/u-boot/custodians/u-boot-cfi-flash
> (2021-08-11 08:31:56 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-x86
> 
> for you to fetch changes up to cedd754484eb929b6f90c60d5d7e6ee458440152:
> 
>   x86: crownbay: Enable CONFIG_SPI_FLASH_SMART_HWCAPS (2021-08-13
> 08:53:49 +0800)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: Pull request: u-boot-rockchip-20210812

2021-08-13 Thread Tom Rini

On Thu, Aug 12, 2021 at 04:39:33PM +0800, Kever Yang wrote:

> Hi Tom,
> 
> Please pull the rockchip updates/fixes:
> - Add Rockchip SFC driver support;
> - DTS sync from kernel;
> - emmc hs400 support for rk3399;
> - Fix for spinore bootdevice and MMC boot order;
> 
> CI:
> https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/8670
> 
> Thanks,
> - Kever
> 
> The following changes since commit 3e5b62f7740530e6f3a2e989f4c361a9d52b:
> 
>   configs: Resync with savedefconfig (2021-08-10 15:08:46 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-rockchip.git 
> tags/u-boot-rockchip-20210812
> 
> for you to fetch changes up to 60df49d22d2586f50bba11eaa59a55f2baa4671f:
> 
>   rockchip: px30: Support configure SFC (2021-08-12 09:34:11 +0800)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 0/3] efi: Minimal revert to rodata change

2021-08-13 Thread Tom Rini

On Mon, Aug 09, 2021 at 12:01:20PM -0400, Tom Rini wrote:
> On Thu, Aug 05, 2021 at 09:46:07AM -0600, Simon Glass wrote:
> 
> > Hi Heinrich,
> > 
> > On Thu, 5 Aug 2021 at 09:29, Heinrich Schuchardt  wrote:
> > >
> > >
> > >
> > > On 02.08.21 16:44, Simon Glass wrote:
> > > > The changes to move from devicetree to rodata take things in the wrong
> > > > direction for various reasons:
> > > >
> > > > - devicetree is where config should be stored
> > >
> > > We are not talking about configuration here at all.
> > 
> > I thought we were talking about the public key. That is run-time
> > config in my book, just like the devicetree itself, which controls all
> > the devices.
> > 
> > >
> > > > - it provides no memory production in any case, particularly when U-Boot
> > >
> > > No clue what you mean by "memory production".
> > 
> > memory protection. But it turns out this is pointless anyway. We
> > discussed it at length in the contributor call. We came down to one
> > issue with the way the firmware is packaged by users (with U-Boot
> > coming from one place and TF-A another). I think Ilias is going to
> > write something up to help get to the bottom of it.
> > 
> > >
> > > >is relocated
> > > > - testing becomes harder, with the suggestion of adding an entire new
> > > >sandbox build just for this
> > >
> > > Having an extra config is not required when putting the certificate into
> > > .rodata.
> > 
> > The certificate should not go in rodata, period. Please just fix it.
> > It use to be fine a few weeks ago so it should not be hard.
> 
> Where are we at here, Heinrich?  Thanks.

Heinrich?

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v1] imx: mkimage_fit_atf: replace @ with hyphen

Hi Tim,

[Adding Marek]

On Tue, Jul 27, 2021 at 6:53 PM Tim Harvey  wrote:

> With respect to moving to binman, I'm all for it but I'm wondering why
> other boards have elected to move from a monolithic flash.bin
> including the SPL and u-boot.itb pre-binman to post-binman where
> flash.bin is just the SPL. I'm not sure why users should be hit with a
> change like that... why are people not having binman produce a
> flash.bin that was equivalent to pre-binman?

Yes, fully agree.

Peng,

After the adoption of binman, both flash.bin and u-boot.itb need to be
flashed to the boot media.

This breaks user experience, distro integration, etc, which is not good.

Can we have binman generate a single bootable flash.bin binary (which
contains flash.bin + u-boot.itb) again
to keep compatibility and avoid breakage?

Thanks,

Fabio Estevam

>
> Best regards,
>
> Tim

[PATCH 2/2] armv8: Ensure EL1&0 VMSA is enabled

2021-08-13 Thread Peter Hoyes

From: Peter Hoyes 

On Armv8-R, the EL1&0 memory system architecture is configurable as a
VMSA or PMSA, and resets to an "architecturally unknown" value.

Add code to armv8_switch_to_el1_m which detects whether the MSA at
EL1&0 is configurable using the id_aa64mmfr0_el1 register MSA fields.
If it is we must ensure the VMSA is enabled so that a rich OS can boot.

The MSA and MSA_FRAC fields are described in the Armv8-R architecture
profile supplement (section G1.3.7):
https://developer.arm.com/documentation/ddi0600/latest/

Signed-off-by: Peter Hoyes 
---
 arch/arm/include/asm/macro.h  | 17 +
 arch/arm/include/asm/system.h | 24 
 2 files changed, 41 insertions(+)

diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h
index e1eefc283f..ecd8221c0d 100644
--- a/arch/arm/include/asm/macro.h
+++ b/arch/arm/include/asm/macro.h
@@ -316,6 +316,23 @@ lr .reqx30
csel\tmp, \tmp2, \tmp, eq
msr hcr_el2, \tmp
 
+   /*
+* Detect whether the system has a configurable memory system
+* architecture at EL1&0
+*/
+   mrs \tmp, id_aa64mmfr0_el1
+   lsr \tmp, \tmp, #48
+   and \tmp, \tmp, #((ID_AA64MMFR0_EL1_MSA_MASK | \
+   ID_AA64MMFR0_EL1_MSA_FRAC_MASK) >> 48)
+   cmp \tmp, #((ID_AA64MMFR0_EL1_MSA_USE_FRAC | \
+   ID_AA64MMFR0_EL1_MSA_FRAC_VMSA) >> 48)
+   bne 2f
+
+   /* Ensure the EL1&0 VMSA is enabled */
+   mov \tmp, #(VTCR_EL2_MSA)
+   msr vtcr_el2, \tmp
+2:
+
/* Return to the EL1_SP1 mode from EL2 */
ldr \tmp, =(SPSR_EL_DEBUG_MASK | SPSR_EL_SERR_MASK |\
SPSR_EL_IRQ_MASK | SPSR_EL_FIQ_MASK |\
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 77aa18909e..e4c11e830a 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -83,6 +83,30 @@
 #define HCR_EL2_RW_AARCH32 (0 << 31) /* Lower levels are AArch32 */
 #define HCR_EL2_HCD_DIS(1 << 29) /* Hypervisor Call disabled   
  */
 
+/*
+ * VTCR_EL2 bits definitions
+ */
+#define VTCR_EL2_MSA   (1 << 31) /* EL1&0 memory architecture*/
+
+/*
+ * ID_AA64MMFR0_EL1 bits definitions
+ */
+#define ID_AA64MMFR0_EL1_MSA_FRAC_MASK (0xFUL << 52) /* Memory system
+architecture
+frac */
+#define ID_AA64MMFR0_EL1_MSA_FRAC_VMSA (0x2UL << 52) /* EL1&0 supports
+VMSA */
+#define ID_AA64MMFR0_EL1_MSA_FRAC_PMSA (0x1UL << 52) /* EL1&0 only
+supports PMSA*/
+#define ID_AA64MMFR0_EL1_MSA_FRAC_NO_PMSA  (0x0UL << 52) /* No PMSA
+support  */
+#define ID_AA64MMFR0_EL1_MSA_MASK  (0xFUL << 48) /* Memory system
+architecture */
+#define ID_AA64MMFR0_EL1_MSA_USE_FRAC  (0xFUL << 48) /* Use MSA_FRAC */
+#define ID_AA64MMFR0_EL1_MSA_VMSA  (0x0UL << 48) /* Memory system
+architecture
+is VMSA  */
+
 /*
  * ID_AA64ISAR1_EL1 bits definitions
  */
-- 
2.25.1

[PATCH 1/2] armv8: Disable pointer authentication traps for EL1

2021-08-13 Thread Peter Hoyes

From: Peter Hoyes 

The use of ARMv8.3 pointer authentication (PAuth) is governed by fields
in HCR_EL2, which trigger a 'trap to EL2' if not enabled. The reset
value of these fields is 'architecturally unknown' so we must ensure
that the fields are enabled (to disable the traps) if we are entering
the kernel at EL1.

The APK field disables PAuth instruction traps and the API field
disables PAuth register traps

Add code to disable the traps in armv8_switch_to_el1_m. Prior to doing
so, it checks fields in the ID_AA64ISAR1_EL1 register to ensure pointer
authentication is supported by the hardware.

The runtime checks require a second temporary register, so add this to
the EL1 transition macro signature and update 2 call sites.

Signed-off-by: Peter Hoyes 
---
 arch/arm/cpu/armv8/fsl-layerscape/spintable.S |  2 +-
 arch/arm/cpu/armv8/transition.S   |  2 +-
 arch/arm/include/asm/macro.h  | 11 +--
 arch/arm/include/asm/system.h | 15 +++
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spintable.S 
b/arch/arm/cpu/armv8/fsl-layerscape/spintable.S
index 363ded03e6..d6bd188459 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/spintable.S
+++ b/arch/arm/cpu/armv8/fsl-layerscape/spintable.S
@@ -93,7 +93,7 @@ __secondary_boot_func:
 4:
 #ifdef CONFIG_ARMV8_SWITCH_TO_EL1
switch_el x7, _dead_loop, 0f, _dead_loop
-0: armv8_switch_to_el1_m x4, x6, x7
+0: armv8_switch_to_el1_m x4, x6, x7, x9
 #else
switch_el x7, 0f, _dead_loop, _dead_loop
 0: armv8_switch_to_el2_m x4, x6, x7
diff --git a/arch/arm/cpu/armv8/transition.S b/arch/arm/cpu/armv8/transition.S
index a31af4ffc8..9dbdff3a4f 100644
--- a/arch/arm/cpu/armv8/transition.S
+++ b/arch/arm/cpu/armv8/transition.S
@@ -40,7 +40,7 @@ ENTRY(armv8_switch_to_el1)
 * now, jump to the address saved in x4.
 */
br x4
-1: armv8_switch_to_el1_m x4, x5, x6
+1: armv8_switch_to_el1_m x4, x5, x6, x7
 ENDPROC(armv8_switch_to_el1)
 .popsection
 
diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h
index 485310d660..e1eefc283f 100644
--- a/arch/arm/include/asm/macro.h
+++ b/arch/arm/include/asm/macro.h
@@ -256,7 +256,7 @@ lr  .reqx30
  * For loading 64-bit OS, x0 is physical address to the FDT blob.
  * They will be passed to the guest.
  */
-.macro armv8_switch_to_el1_m, ep, flag, tmp
+.macro armv8_switch_to_el1_m, ep, flag, tmp, tmp2
/* Initialize Generic Timers */
mrs \tmp, cnthctl_el2
/* Enable EL1 access to timers */
@@ -306,7 +306,14 @@ lr .reqx30
b.eq1f
 
/* Initialize HCR_EL2 */
-   ldr \tmp, =(HCR_EL2_RW_AARCH64 | HCR_EL2_HCD_DIS)
+   /* Only disable PAuth traps if PAuth is supported */
+   mrs \tmp, id_aa64isar1_el1
+   ldr \tmp2, =(ID_AA64ISAR1_EL1_GPI | ID_AA64ISAR1_EL1_GPA | \
+ ID_AA64ISAR1_EL1_API | ID_AA64ISAR1_EL1_APA)
+   tst \tmp, \tmp2
+   mov \tmp2, #(HCR_EL2_RW_AARCH64 | HCR_EL2_HCD_DIS)
+   orr \tmp, \tmp2, #(HCR_EL2_APK | HCR_EL2_API)
+   csel\tmp, \tmp2, \tmp, eq
msr hcr_el2, \tmp
 
/* Return to the EL1_SP1 mode from EL2 */
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 8b3a54e64c..77aa18909e 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -75,10 +75,25 @@
 /*
  * HCR_EL2 bits definitions
  */
+#define HCR_EL2_API(1 << 41) /* Trap pointer authentication
+instructions */
+#define HCR_EL2_APK(1 << 40) /* Trap pointer authentication
+key access   */
 #define HCR_EL2_RW_AARCH64 (1 << 31) /* EL1 is AArch64   */
 #define HCR_EL2_RW_AARCH32 (0 << 31) /* Lower levels are AArch32 */
 #define HCR_EL2_HCD_DIS(1 << 29) /* Hypervisor Call disabled   
  */
 
+/*
+ * ID_AA64ISAR1_EL1 bits definitions
+ */
+#define ID_AA64ISAR1_EL1_GPI   (0xF << 28) /* Implementation-defined generic
+  code auth algorithm*/
+#define ID_AA64ISAR1_EL1_GPA   (0xF << 24) /* QARMA generic code auth
+  algorithm  */
+#define ID_AA64ISAR1_EL1_API   (0xF << 8)  /* Implementation-defined address
+  auth algorithm */
+#define ID_AA64ISAR1_EL1_APA   (0xF << 4)  /* QARMA address auth algorithm   */
+
 /*
  * ID_AA64PFR0_EL1 bits definitions
  */
-- 
2.25.1

Re: [PATCH 2/2] imx8mm-cl-iot-gate: Add documentation

2021-08-13 Thread Paul Liu

Hi Fabio,

Did you use the boot partition?

I mean, why don't we use "mmc partconf 0 0 1 0"
So that we can flash u-boot to partition 1 and leave partition 0 for distro
install.

The following sequence should work.

init setup
1. mmc dev 2
2. mmc partconf 0 0 1 0

flash u-boot
1. tftp ${loadaddr} flash.bin
2. setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
3. mmc dev 2 1
4. mmc write $loadaddr 0x42 $blkcnt
5.  tftp ${loadaddr} u-boot.itb
6. setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
7. mmc dev 2 1
8. mmc write $loadaddr 0x300 $blkcnt

Yours,
Paul


On Fri, 13 Aug 2021 at 08:59, Fabio Estevam  wrote:

> Add documentation for building and flashing mainline U-Boot
> in the IOT-GATE-iMX8 board.
>
> Signed-off-by: Fabio Estevam 
> ---
>  doc/board/compulab/imx8mm-cl-iot-gate.rst | 84 +++
>  doc/board/index.rst   |  1 +
>  2 files changed, 85 insertions(+)
>  create mode 100644 doc/board/compulab/imx8mm-cl-iot-gate.rst
>
> diff --git a/doc/board/compulab/imx8mm-cl-iot-gate.rst
> b/doc/board/compulab/imx8mm-cl-iot-gate.rst
> new file mode 100644
> index ..b63b8d61f13f
> --- /dev/null
> +++ b/doc/board/compulab/imx8mm-cl-iot-gate.rst
> @@ -0,0 +1,83 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +IOT-GATE-iMX8
> +=
> +
> +U-Boot for Compulab i.MX8MM IoT Gateway board.
> +
> +Quick Start
> +---
> +
> +- Build the ARM Trusted firmware binary
> +- Get the DDR firmwares
> +- Build U-Boot
> +- Flash U-Boot into the eMMC
> +
> +Get and build the ARM Trusted firmware
> +--
> +
> +Note: builddir is U-Boot build directory (source directory for in-tree
> builds).
> +
> +Get mainline ATF:
> +
> +.. code-block:: bash
> +
> +   $ git clone https://github.com/ARM-software/arm-trusted-firmware.git
> +   $ cd arm-trusted-firmware
> +   $ git checkout v2.5
> +
> +Generate the bl31.bin ATF binary:
> +
> +.. code-block:: bash
> +
> +   $ export CROSS_COMPILE=aarch64-poky-linux-
> +   $ make PLAT=imx8mm IMX_BOOT_UART_BASE=0x3088 bl31
> +   $ cp build/imx8mm/release/bl31.bin $(builddir)
> +
> +Get the DDR firmwares
> +-
> +
> +.. code-block:: bash
> +
> +   $ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-8.9.bin
> +   $ chmod +x firmware-imx-8.9.bin
> +   $ ./firmware-imx-8.9
> +   $ cp firmware-imx-8.9/firmware/ddr/synopsys/lpddr4*.bin $(builddir)
> +
> +Build U-Boot
> +
> +
> +.. code-block:: bash
> +
> +   $ export CROSS_COMPILE=aarch64-poky-linux-
> +   $ make imx8mm-cl-iot-gate_defconfig
> +   $ export ATF_LOAD_ADDR=0x92
> +   $ make
> +
> +This will result in two binaries: flash.bin and u-boot.itb.
> +
> +Flash U-Boot into the eMMC
> +--
> +
> +Make sure to have access to the IOTG-ACC-M2SD adapter to recover
> +the board in case something goes wrong. More details at:
> +
> https://mediawiki.compulab.com/w/index.php?title=IOT-GATE-iMX8_and_SBC-IOT-iMX8:_U-Boot:_Recovery
> +
> +The flash.bin and u-boot.itb binaries need to be flashed into the eMMC at
> +offset 33K and 384K, respectively.
> +
> +These binaries can be transferred from the host PC to the board running
> +U-Boot via TFTP:
> +
> +.. code-block:: bash
> +
> +   => mmc dev 2
> +   => mmc partconf 2 1 7 0 (This is only needed to be done for the first
> time)
> +
> +   => tftp $loadaddr flash.bin
> +   => setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
> +   => mmc write $loadaddr 0x42 $blkcnt
> +
> +   => tftp $loadaddr u-boot.itb
> +   => setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
> +   => mmc write $loadaddr 0x300 $blkcnt
> diff --git a/doc/board/index.rst b/doc/board/index.rst
> index 9e9097889161..5c5420f3d75a 100644
> --- a/doc/board/index.rst
> +++ b/doc/board/index.rst
> @@ -11,6 +11,7 @@ Board-specific doc
> AndesTech/index
> amlogic/index
> atmel/index
> +   compulab/index
> congatec/index
> coreboot/index
> emulation/index
> --
> 2.25.1
>
>

Re: [PATCH 1/2] imx8mm-cl-iot-gate: Do not build fip.bin by default

2021-08-13 Thread Paul Liu

Hi Fabio,

Can we think of a way to keep this fip.bin feature? This is part of the
SystemReady IR certificate for this Compulab board. [1]

[1]
https://developer.arm.com/architectures/system-architectures/arm-systemready/ir


Yours,
Paul





On Fri, 13 Aug 2021 at 08:59, Fabio Estevam  wrote:

> When trying to build the imx8mm-cl-iot-gate_defconfig target there is a
> build error due to the missing 'fip.bin'.
>
> To make the build process more consistent with other i.MX8M boards,
> do not build fip.bin by default.
>
> Signed-off-by: Fabio Estevam 
> ---
>  arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi | 14 +-
>  1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi
> b/arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi
> index 3226a244a97e..00927c157449 100644
> --- a/arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi
> @@ -217,18 +217,6 @@
> };
> };
>
> -   fip {
> -   description = "Trusted Firmware
> FIP";
> -   type = "firmware";
> -   arch = "arm64";
> -   compression = "none";
> -   load = <0x4031>;
> -
> -   fip_blob: blob-ext{
> -   filename = "fip.bin";
> -   };
> -   };
> -
> fdt {
> description = "NAME";
> type = "flat_dt";
> @@ -246,7 +234,7 @@
> conf {
> description = "NAME";
> firmware = "uboot";
> -   loadables = "atf", "fip";
> +   loadables = "atf";
> fdt = "fdt";
> };
> };
> --
> 2.25.1
>
>

[PATCH 4/4] mtd: spi-nor-core: Add support for Macronix Octal flash

Adding Macronix Octal flash for Octal DTR support.

The octaflash series can be divided into the following types:

MX25 series : Serial NOR Flash.
MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb)
LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation.
LW/UW series : Support simultaneous Read-while-Write operation in multiple
   bank architecture. Read-while-write feature which means read
   data one bank while another bank is programing or erasing.

MX25LM : 3.0V Octal I/O
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf

MX25UM : 1.8V Octal I/O
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7525/MX25UM51245G%20Extreme%20Speed,%201.8V,%20512Mb,%20v1.0.pdf

MX66LM : 3.0V Octal I/O with stacked die
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf

MX66UM : 1.8V Octal I/O with stacked die
 
-https://www.mxic.com.tw/Lists/Datasheet/Attachments/7721/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf

MX25LW : 3.0V Octal I/O with Read-while-Write
MX25UW : 1.8V Octal I/O with Read-while-Write
MX66LW : 3.0V Octal I/O with Read-while-Write and stack die
MX66UW : 1.8V Octal I/O with Read-while-Write and stack die

About LW/UW series, please contact us freely if you have any
questions. For adding Octal NOR Flash IDs, we have validated
each Flash on plateform zynq-picozed.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/spi-nor-ids.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index cb3a08872d..5c13ea3a78 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -169,7 +169,27 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("mx66l1g45g",  0xc2201b, 0, 64 * 1024, 2048, SECT_4K | 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("mx25l1633e", 0xc22415, 0, 64 * 1024,   32, SPI_NOR_QUAD_READ | 
SPI_NOR_4B_OPCODES | SECT_4K) },
{ INFO("mx25r6435f", 0xc22817, 0, 64 * 1024,   128,  SECT_4K) },
-   { INFO("mx66uw2g345g", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66uw2g345gx0", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66lm1g45g",0xc2853b, 0, 32 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25lm51245g",   0xc2853a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25lw51245g",   0xc2863a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25lm25645g",   0xc28539, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66um2g45g",0xc2803c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66uw2g345g",   0xc2843c, 0, 64 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66um1g45g",0xc2803b, 0, 32 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx66uw1g45g",0xc2813b, 0, 32 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25um51245g",   0xc2803a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw51245g",   0xc2813a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw51345g",   0xc2843a, 0, 16 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25um25645g",   0xc28039, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw25645g",   0xc28139, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25um25345g",   0xc28339, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw25345g",   0xc28439, 0, 8 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw12845g",   0xc28138, 0, 4 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw12a45g",   0xc28938, 0, 4 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw12345g",   0xc28438, 0, 4 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw6445g",0xc28137, 0, 2 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
+   { INFO("mx25uw6345g",0xc28437, 0, 2 * 1024, 4096, SECT_4K | 
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
 #endif
 
 #ifdef CONFIG_SPI_FLASH_STMICRO/* STMICRO */
-- 
2.17.1

[PATCH 3/4] mtd: spi-nor-core: set 4byte opcode when possible

Following linux kernel to check address width and 4byte flag to enable
4byte opcode setting.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/spi-nor-core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index be6c58ad40..1bddfc10a2 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3919,6 +3919,10 @@ int spi_nor_scan(struct spi_nor *nor)
return -EINVAL;
}
 
+   /* Set 4byte opcodes when possible. */
+   if (nor->addr_width == 4 && info->flags & SPI_NOR_4B_OPCODES)
+   spi_nor_set_4byte_opcodes(nor, info);
+
/* Send all the required SPI flash commands to initialize device */
ret = spi_nor_init(nor);
if (ret)
-- 
2.17.1

[PATCH 2/4] mtd: spi-nor-core: Adding different type of command extension in Soft Reset

Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from 8D-8D-8D
in the begging of probe.

Command extension type is not standardized across flash vendors in DTR mode.

For suiting different vendor flash devices, having second times Softreset with
different types is clumsy but useful in the begging of probe.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/spi-nor-core.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 6b195b1fd3..be6c58ad40 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3684,6 +3684,36 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
 */
udelay(SPI_NOR_SRST_SLEEP_LEN);
 
+   /* Manufacturers with different command extension type. For suitting
+* different flash devices, using command extension type is equal 
"INVERT"
+* when second time Software Reset.
+*/
+
+   nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
+   op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0),
+   SPI_MEM_OP_NO_DUMMY,
+   SPI_MEM_OP_NO_ADDR,
+   SPI_MEM_OP_NO_DATA);
+   spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+   ret = spi_mem_exec_op(nor->spi, &op);
+   if (ret) {
+   dev_warn(nor->dev, "Software reset enable failed: %d\n", ret);
+   goto out;
+   }
+
+   op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 0),
+   SPI_MEM_OP_NO_DUMMY,
+   SPI_MEM_OP_NO_ADDR,
+   SPI_MEM_OP_NO_DATA);
+   spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+   ret = spi_mem_exec_op(nor->spi, &op);
+   if (ret) {
+   dev_warn(nor->dev, "Software reset failed: %d\n", ret);
+   goto out;
+   }
+
+   udelay(SPI_NOR_SRST_SLEEP_LEN);
+
 out:
nor->cmd_ext_type = ext;
return ret;
-- 
2.17.1

[PATCH 1/4] mtd: spi-nor: macronix: add support for Macronix octaflash

Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding
Macronix flash in Octal DTR mode.
Enable Octal DTR mode with 20 dummy cycles to allow running at the
maximum supported frequency.

Signed-off-by: JaimeLiao 
---
 drivers/mtd/spi/spi-nor-core.c | 75 ++
 include/linux/mtd/spi-nor.h| 13 +-
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d5d905fa5a..6b195b1fd3 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3489,6 +3489,77 @@ static struct spi_nor_fixups mt35xu512aba_fixups = {
 };
 #endif /* CONFIG_SPI_FLASH_MT35XU */
 
+#ifdef CONFIG_SPI_FLASH_MACRONIX
+/**
+ * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in 
Configuration Register 2.
+ * @nor:   pointer to a 'struct spi_nor'
+ *
+ * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2.
+ *
+ * bit 2 of  Configuration Register 2 is the DOPI bit for Macronix like OPI 
memories.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor)
+{
+   struct spi_mem_op op;
+   int ret;
+   u8 buf;
+
+   write_enable(nor);
+
+   buf = SPINOR_REG_MXIC_DC_20;
+   op = (struct spi_mem_op)
+   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
+  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1),
+  SPI_MEM_OP_NO_DUMMY,
+  SPI_MEM_OP_DATA_OUT(1, &buf, 1));
+
+   ret = spi_mem_exec_op(nor->spi, &op);
+   if (ret)
+   return ret;
+
+   ret = spi_nor_wait_till_ready(nor);
+   if (ret)
+   return ret;
+
+   nor->read_dummy = MXIC_MAX_DC;
+   write_enable(nor);
+
+   buf = SPINOR_REG_MXIC_OPI_DTR_EN;
+   op = (struct spi_mem_op)
+   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
+  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1),
+  SPI_MEM_OP_NO_DUMMY,
+  SPI_MEM_OP_DATA_OUT(1, &buf, 1));
+
+   ret = spi_mem_exec_op(nor->spi, &op);
+   if (ret) {
+   dev_err(nor->dev, "Failed to enable octal DTR mode\n");
+   return ret;
+   }
+   nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
+
+   return 0;
+}
+
+static void macronix_default_init(struct spi_nor *nor)
+{
+   nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
+}
+
+static void macronix_post_sfdp_fixup(struct spi_nor *nor,
+struct spi_nor_flash_parameter *params)
+{
+   params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
+}
+
+static struct spi_nor_fixups macronix_fixups = {
+   .default_init = macronix_default_init,
+   .post_sfdp = macronix_post_sfdp_fixup,
+};
+#endif /* CONFIG_SPI_FLASH_MACRONIX */
+
 /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
  * @nor: pointer to a 'struct spi_nor'
  *
@@ -3655,6 +3726,10 @@ void spi_nor_set_fixups(struct spi_nor *nor)
if (!strcmp(nor->info->name, "mt35xu512aba"))
nor->fixups = &mt35xu512aba_fixups;
 #endif
+
+#ifdef CONFIG_SPI_FLASH_MACRONIX
+   nor->fixups = ¯onix_fixups;
+#endif
 }
 
 int spi_nor_scan(struct spi_nor *nor)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 7ddc4ba2bf..2ad579f66d 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -116,8 +116,17 @@
 #define XSR_RDYBIT(7)  /* Ready */
 
 /* Used for Macronix and Winbond flashes. */
-#define SPINOR_OP_EN4B 0xb7/* Enter 4-byte mode */
-#define SPINOR_OP_EX4B 0xe9/* Exit 4-byte mode */
+#define SPINOR_OP_EN4B 0xb7/* Enter 4-byte mode */
+#define SPINOR_OP_EX4B 0xe9/* Exit 4-byte mode */
+#define SPINOR_OP_RD_CR2   0x71/* Read configuration 
register 2 */
+#define SPINOR_OP_WR_CR2   0x72/* Write configuration 
register 2 */
+#define SPINOR_OP_MXIC_DTR_RD  0xee/* Fast Read opcode in 
DTR mode */
+#define SPINOR_REG_MXIC_CR2_MODE   0x  /* For setting octal 
DTR mode */
+#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */
+#define SPINOR_REG_MXIC_OPI_DTR_DIS0x1 /* Disable Octal DTR */
+#define SPINOR_REG_MXIC_CR2_DC 0x0300  /* For setting dummy 
cycles */
+#define SPINOR_REG_MXIC_DC_20  0x0 /* Setting dummy cycles 
to 20 */
+#define MXIC_MAX_DC20  /* Maximum value of 
dummy cycles */
 
 /* Used for Spansion flashes only. */
 #define SPINOR_OP_BRWR 0x17/* Bank register write */
-- 
2.17.1

[PATCH 0/4] Add octal DTR support for Macronix flash

This series add support for Macronix octal DTR flash, add second time
Softreset with "INVERT" command extension type and follow linux kernel
to enable 4byte opcode when possible.

JaimeLiao (4):
  mtd: spi-nor: macronix: add support for Macronix octaflash
  mtd: spi-nor-core: Adding different type of command extension in Soft
Reset
  mtd: spi-nor-core: set 4byte opcode when possible
  mtd: spi-nor-core: Add support for Macronix Octal flash

 drivers/mtd/spi/spi-nor-core.c | 109 +
 drivers/mtd/spi/spi-nor-ids.c  |  22 ++-
 include/linux/mtd/spi-nor.h|  13 +++-
 3 files changed, 141 insertions(+), 3 deletions(-)

-- 
2.17.1

Re: [PATCH] rpi: Conditionally add simple-framebuffer node

2021-08-13 Thread Ivan T . Ivanov

Hi,

Quoting Ivan T. Ivanov (2021-08-10 17:31:14)
> It appears that RPi firmware has already added framebuffer
> node under /chosen, at least on RPi 2 versions. So check
> for this and don't add duplicate node.
> 
> Signed-off-by: Ivan T. Ivanov 
> ---
>  board/raspberrypi/rpi/rpi.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index df52a4689f..372b26b6f2 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -497,12 +497,11 @@ void *board_fdt_blob_setup(void)
>  
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
> -   /*
> -* For now, we simply always add the simplefb DT node. Later, we
> -* should be more intelligent, and e.g. only do this if no enabled DT
> -* node exists for the "real" graphics driver.
> -*/
> -   lcd_dt_simplefb_add_node(blob);
> +   int node;
> +
> +   node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer");
> +   if (node < 0)
> +   lcd_dt_simplefb_add_node(blob);
>  
>  #ifdef CONFIG_EFI_LOADER
> /* Reserve the spin table */

Could someone take a look into this fix, please.

BTW, original message still awaits list moderator approval.

Thank you!
Ivan

Re: [PATCH 1/2] imx8mm-cl-iot-gate: Do not build fip.bin by default


Hi Paul,

On 13/08/2021 06:59, Paul Liu wrote:

Hi Fabio,

Can we think of a way to keep this fip.bin feature? This is part of
the SystemReady IR certificate for this Compulab board. [1]

[1]
https://developer.arm.com/architectures/system-architectures/arm-systemready/ir


i.MX8MM EVK is also listed there and we don't use fip.bin on this 
platform (and

not on any other i.MX8MM platform in mainline U-Boot).

If you really want to use fip.bin, could you please let me know the 
exact

build procedure for AT-F? Do you use mainline ATF?

As a user of the IOT-GATE-iMX8, I would just like to be able to flash 
mainline

U-Boot and move forward. Even better if I could use the same procedure
that has been used on other i.MX8MM boards.

Regards,

Fabio Estevam
--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-60 Fax: (+49)-8142-66989-80 Email: 
feste...@denx.de

Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

2021-08-13 Thread Rasmus Villemoes

On 12/08/2021 08.50, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <20210811124800.2593226-10-rasmus.villem...@prevas.dk> you wrote:
>>
>> +ret = uclass_get(UCLASS_WDT, &uc);
>> +if (ret) {
>> +log_debug("Error getting UCLASS_WDT: %d\n", ret);
>> +return 0;
>> +}
> 
> Here the error goes silent, so we should fix the callers to report
> it.

The caller (singular) is the initr sequence, so returning an error is
effectively the same as halting the boot process, and as I've already
explained, I'm not going to change the semantics of initr_watchdog in
this regard.

Note that, as I've also already explained, the code which this is
replacing would also fail if uclass_get() fails (there are uclass_get()s
done inside the uclass_get_device_by_seq() and uclass_get_device()), and
would similarly just turn an error from one of those into "return 0".

Feel free to submit a patch if you feel a change in this area is in
order. That's completely unrelated to what these patches are trying to
achieve.

>> +uclass_foreach_dev(dev, uc) {
>> +ret = device_probe(dev);
>> +if (ret) {
>> +log_debug("Error probing %s: %d\n", dev->name, ret);
>> +continue;
>>  }
> 
> Here the situation is different.  The probing error is never
> reported anywhere.  Is it really a normal condition that a
> device_probe() fails here?

No, it is not a normal condition. I added the log_debug() after a
request from Simon.

Rasmus

[PATCH] serial: a37xx: Remove CONFIG_DEBUG_UART_SHIFT options

Armada 37xx serial driver does not use CONFIG_DEBUG_UART_SHIFT.

So do not define any bogus value for CONFIG_DEBUG_UART_SHIFT option in any
Armada 37xx defconfig file.

Signed-off-by: Pali Rohár 
---
 configs/mvebu_db-88f3720_defconfig  | 1 -
 configs/mvebu_espressobin-88f3720_defconfig | 1 -
 configs/turris_mox_defconfig| 1 -
 configs/uDPU_defconfig  | 1 -
 4 files changed, 4 deletions(-)

diff --git a/configs/mvebu_db-88f3720_defconfig 
b/configs/mvebu_db-88f3720_defconfig
index bc92fdb8ee8c..eb50afc0f381 100644
--- a/configs/mvebu_db-88f3720_defconfig
+++ b/configs/mvebu_db-88f3720_defconfig
@@ -63,7 +63,6 @@ CONFIG_PHY=y
 CONFIG_MVEBU_COMPHY_SUPPORT=y
 CONFIG_PINCTRL=y
 CONFIG_PINCTRL_ARMADA_37XX=y
-CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_MVEBU_A3700_UART=y
 CONFIG_MVEBU_A3700_SPI=y
diff --git a/configs/mvebu_espressobin-88f3720_defconfig 
b/configs/mvebu_espressobin-88f3720_defconfig
index 4003a25346ce..9641c02d9317 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -79,7 +79,6 @@ CONFIG_MVEBU_COMPHY_SUPPORT=y
 CONFIG_PINCTRL=y
 CONFIG_PINCTRL_ARMADA_37XX=y
 CONFIG_DM_REGULATOR_GPIO=y
-CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_MVEBU_A3700_UART=y
 CONFIG_MVEBU_A3700_SPI=y
diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig
index fc5e1fc3766f..40f975ead314 100644
--- a/configs/turris_mox_defconfig
+++ b/configs/turris_mox_defconfig
@@ -87,7 +87,6 @@ CONFIG_DM_RTC=y
 CONFIG_RTC_DS1307=y
 CONFIG_SCSI=y
 CONFIG_DM_SCSI=y
-CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_MVEBU_A3700_UART=y
 CONFIG_MVEBU_A3700_SPI=y
diff --git a/configs/uDPU_defconfig b/configs/uDPU_defconfig
index 3e6bb32cb88b..1ea3aad5ff2a 100644
--- a/configs/uDPU_defconfig
+++ b/configs/uDPU_defconfig
@@ -79,7 +79,6 @@ CONFIG_PINCTRL=y
 CONFIG_PINCTRL_ARMADA_37XX=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
-CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_MVEBU_A3700_UART=y
 CONFIG_MVEBU_A3700_SPI=y
-- 
2.20.1

[PATCH 3/3] arm: mvebu: turris_omnia: Enable NVMe support

PCIe-based NVMe SSD disks in M.2 2230/2242/2260 form-factor can be
connected to Turris Omnia mPCIe slot via passive M.2 <--> mPCIe adapter.

So enable PCIe NVMe drivers.

Signed-off-by: Pali Rohár 
---
 configs/turris_omnia_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
index cd443ceb300f..2acc4ada4123 100644
--- a/configs/turris_omnia_defconfig
+++ b/configs/turris_omnia_defconfig
@@ -77,6 +77,7 @@ CONFIG_PHY_MARVELL=y
 CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
+CONFIG_NVME=y
 CONFIG_PCI=y
 CONFIG_PCI_MVEBU=y
 CONFIG_DM_RTC=y
-- 
2.20.1

[PATCH 1/3] arm: mvebu: espressobin: Enable also SATA support via PCIe

Espressobin has one on-board SATA port which is connected directly to CPU.

More SATA disks can be connected via mPCIe add-in card with PCIe-SATA
controller.

So enable required SATA AHCI PCIe drivers in defconfig file.

Signed-off-by: Pali Rohár 
---
 configs/mvebu_espressobin-88f3720_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/mvebu_espressobin-88f3720_defconfig 
b/configs/mvebu_espressobin-88f3720_defconfig
index c8ae0cf6109a..4003a25346ce 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -30,6 +30,7 @@ CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_MTD=y
 CONFIG_CMD_PCI=y
+CONFIG_CMD_SATA=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_WDT=y
@@ -46,6 +47,7 @@ CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_AHCI_PCI=y
 CONFIG_AHCI_MVEBU=y
 CONFIG_CLK=y
 CONFIG_CLK_MVEBU=y
-- 
2.20.1

[PATCH 2/3] arm: mvebu: turris_mox: Enable SATA support

SATA disks could be connected via mPCIe add-in card with PCIe-SATA
controller into Mox-B or Mox-G module.

Signed-off-by: Pali Rohár 
---
 configs/turris_mox_defconfig | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig
index c19b8379c32a..fc5e1fc3766f 100644
--- a/configs/turris_mox_defconfig
+++ b/configs/turris_mox_defconfig
@@ -12,6 +12,7 @@ CONFIG_DM_GPIO=y
 CONFIG_DEFAULT_DEVICE_TREE="armada-3720-turris-mox"
 CONFIG_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
 CONFIG_OF_BOARD_FIXUP=y
 CONFIG_DISTRO_DEFAULTS=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
@@ -32,6 +33,7 @@ CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_MTD=y
 CONFIG_CMD_PCI=y
+CONFIG_CMD_SATA=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_WDT=y
@@ -47,6 +49,8 @@ CONFIG_MAC_PARTITION=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_SCSI_AHCI=y
+CONFIG_AHCI_PCI=y
 CONFIG_BUTTON=y
 CONFIG_BUTTON_GPIO=y
 CONFIG_CLK=y
@@ -81,6 +85,8 @@ CONFIG_PINCTRL_ARMADA_37XX=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1307=y
+CONFIG_SCSI=y
+CONFIG_DM_SCSI=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_MVEBU_A3700_UART=y
-- 
2.20.1

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Friday 13 August 2021 12:33:25 Luka Kovacic wrote:
> On Fri, Aug 13, 2021 at 12:22 PM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 12:03:57 Luka Kovacic wrote:
> > > Hello Pali,
> > >
> > > On Fri, Aug 13, 2021 at 11:27 AM Pali Rohár  wrote:
> > > >
> > > > On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> > > > > Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> > > > > Technologies, Inc.
> > > > >
> > > > > The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> > > > > Peripherals:
> > > > >  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 
> > > > > switch)
> > > > >  - RTC clock (PCF8563)
> > > > >  - USB 3.0 port
> > > > >  - USB 2.0 port
> > > > >  - 4x LED
> > > > >  - UART over Micro-USB
> > > > >  - M.2 slot (2280)
> > > > >  - Mini PCI-E slot
> > > > >
> > > > > Additionally, automatic import of the Marvell hw_info parameters is
> > > > > enabled via the recently added mac command for A37XX platforms.
> > > > > The parameters stored in Marvell hw_info are usually the board serial
> > > > > number and MAC addresses.
> > > > >
> > > > > Signed-off-by: Luka Kovacic 
> > > > > Cc: Luka Perkov 
> > > > > Cc: Robert Marko 
> > > > > ---
> > > > >  arch/arm/dts/Makefile |   1 +
> > > > >  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> > > > >  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> > > > >  arch/arm/dts/armada-3720-espressobin.dtsi | 210 
> > > > > ++
> > > > >  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> > > > >  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> > > > >  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> > > > >  7 files changed, 514 insertions(+), 203 deletions(-)
> > > > >  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> > > > >  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> > > > >  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> > > > ...
> > > > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c 
> > > > > b/board/Marvell/mvebu_armada-37xx/board.c
> > > > > index 2de9c2ac17..21c1eb7b22 100644
> > > > > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > > > > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > > > > @@ -11,6 +11,7 @@
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > > +#include 
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > > @@ -55,6 +56,15 @@ DECLARE_GLOBAL_DATA_PTR;
> > > > >  #define MVEBU_G2_SMI_PHY_CMD_REG (24)
> > > > >  #define MVEBU_G2_SMI_PHY_DATA_REG(25)
> > > > >
> > > > > +/* Marvell 88E1512 */
> > > > > +#define MII_MARVELL_PHY_PAGE 22
> > > > > +
> > > > > +#define MV88E1512_GENERAL_CTRL   20
> > > > > +#define MV88E1512_MODE_SGMII 1
> > > > > +#define MV88E1512_RESET_OFFS 15
> > > > > +
> > > > > +#define ULTRA_MV88E1512_PHYADDR  0x1
> > > > > +
> > > > >  /*
> > > > >   * Memory Controller Registers
> > > > >   *
> > > > > @@ -282,12 +292,68 @@ static int mii_multi_chip_mode_write(struct 
> > > > > mii_dev *bus, int dev_smi_addr,
> > > > >   return 0;
> > > > >  }
> > > > >
> > > > > -/* Bring-up board-specific network stuff */
> > > > > -int board_network_enable(struct mii_dev *bus)
> > > > > +void force_phy_88e1512_sgmii_to_copper(u16 devaddr)
> > > > >  {
> > > > > - if (!of_machine_is_compatible("globalscale,espressobin"))
> > > > > - return 0;
> > > > > + const char *name;
> > > > > + u16 reg;
> > > > > +
> > > > > + name = miiphy_get_current_dev();
> > > > > + if (name) {
> > > >
> > > > It is possible that phy is not available? As you are calling code below
> > > > only in case name is not-NULL.
> > >
> > > Well, according to common/miiphyutil.c, it could also happen that it's 
> > > NULL.
> >
> > I see. But if it happens, is not it fatal error for this board? If name
> > is NULL then you cannot correctly configure board correctly, right?
> >
> > > >
> > > > > + /* SGMII-to-Copper mode initialization */
> > > > > +
> > > > > + /* Select page 18 */
> > > > > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0x12);
> > > > > + /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > > > > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, 
> > > > > ®);
> > > > > + reg &= ~0x7;
> > > > > + reg |= MV88E1512_MODE_SGMII;
> > > > > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, 
> > > > > reg);
> > > > > + /* PHY reset is necessary after changing MODE[2:0] */
> > > > > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, 
> > > > > ®);
> > > > > + reg |= 1 << MV88E1512_RESET_OFFS;
> > > > > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, 
> > > > > reg);
> > > > > + /* Reset page sel

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Friday 13 August 2021 12:43:47 Luka Kovacic wrote:
> On Fri, Aug 13, 2021 at 12:29 PM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > > diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig 
> > > b/board/Marvell/mvebu_armada-37xx/Kconfig
> > > new file mode 100644
> > > index 00..b84dd20023
> > > --- /dev/null
> > > +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> > > @@ -0,0 +1,29 @@
> > > +menu "Marvell Armada 37xx configuration"
> > > +depends on TARGET_MVEBU_ARMADA_37XX
> > > +
> > > +config MVEBU_MAC_HW_INFO
> > > + bool "Marvell hw_info (mac) support"
> > > + depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> > > + default n
> > > + help
> > > +   Enable loading of the Marvell hw_info parameters from the
> > > +   SPI flash hw_info area. Parameters (usually the board serial
> > > +   number and MAC addresses) are then imported into the
> > > +   existing U-Boot environment.
> > > +   Implementation of this command is compatible with the
> > > +   original Marvell U-Boot command. Reading and writing is
> > > +   supported.
> > > +   EEPROM config pattern and checksum aren't supported.
> > > +   After enabled, these parameters are managed from the common
> > > +   U-Boot mac command.
> > > +
> > > +config MVEBU_MAC_HW_INFO_OFFSET
> > > + hex "Marvell hw_info (mac) SPI flash offset"
> > > + depends on MVEBU_MAC_HW_INFO
> > > + default 0x3E
> > > + help
> > > +   This option defines the SPI flash offset of the Marvell
> > > +   hw_info area. This defaults to 0x3E on most Armada
> > > +   A3720 platforms.
> >
> > Just a question, cannot we load this offset from DTS? In DTS are already
> > specified SPI partitions, so this could eliminate need for defining this
> > offset at two places. But I really do not know at which time is this
> > code called, if DTB is available at this time or not.
> 
> The code is called right after cpu_secondary_init_r.
> I'm not sure, there also some other values, which are hard-coded and
> so far I didn't really see any other possible offset.
> 
> Are you aware of any other relevant board with the Marvell hw_info
> parameters?

I do not know any other board which uses this hw_info. And because you
adding config option for address, I though that there are more boards
with different addresses... And so I was thinking if it cannot be loaded
fro DTS.

I can check espressobin v5 if there is not some "hidden" hw_info stuff
somewhere...

> >
> > > +endmenu
> 
> Kind regards,
> Luka

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Friday 13 August 2021 12:25:46 Luka Kovacic wrote:
> Hello Stefan and Pali,
> 
> On Fri, Aug 13, 2021 at 11:58 AM Stefan Roese  wrote:
> >
> > Hi,
> >
> > On 13.08.21 11:54, Pali Rohár wrote:
> > > On Friday 13 August 2021 11:08:08 Luka Kovacic wrote:
> > >> Hello Pali,
> > >>
> > >> On Fri, Aug 13, 2021 at 10:14 AM Pali Rohár  wrote:
> > >>>
> > >>> On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> >  Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> >  Technologies, Inc.
> > 
> >  The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> >  Peripherals:
> >    - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 
> >  switch)
> >    - RTC clock (PCF8563)
> >    - USB 3.0 port
> >    - USB 2.0 port
> >    - 4x LED
> >    - UART over Micro-USB
> >    - M.2 slot (2280)
> >    - Mini PCI-E slot
> > 
> >  Additionally, automatic import of the Marvell hw_info parameters is
> >  enabled via the recently added mac command for A37XX platforms.
> >  The parameters stored in Marvell hw_info are usually the board serial
> >  number and MAC addresses.
> > 
> >  Signed-off-by: Luka Kovacic 
> >  Cc: Luka Perkov 
> >  Cc: Robert Marko 
> >  ---
> >    arch/arm/dts/Makefile |   1 +
> >    .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> >    arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> >    arch/arm/dts/armada-3720-espressobin.dtsi | 210 
> >  ++
> >    board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> >    board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> >    .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> >    7 files changed, 514 insertions(+), 203 deletions(-)
> >    create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> >    create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> >    create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> > >>>
> > >>> Hello Luka! Please look at my comments from previous review:
> > >>> https://lore.kernel.org/u-boot/20210227004852.5urcwnn6uxehuk72@pali/
> > >>>
> > >>> I think it is not a good idea to duplicate espressobin code, specially
> > >>> now when differences between v5, v7, non-emmc and emmc were
> > >>> de-duplicated and correctly detected at runtime. Just use one DTS and
> > >>> one config file and differences can be handled in board code functions
> > >>> "board_fix_fdt" and "board_late_init".
> > >>
> > >> I believe that patching the DTS at runtime diminishes the value of
> > >> device trees in the first case.
> > >>
> > >> As far as the v5, v7, non-emmc and emmc boards go I do understand
> > >> that, as they are physically similar and more or less different revisions
> > >> of the same board.
> > >>
> > >> The ESPRESSOBin Ultra board is completely different from those boards
> > >> physically and so I doesn't make sense to me, why we would merge all
> > >> of them into one device tree.
> > >
> > > See email for reasons:
> > > https://lore.kernel.org/u-boot/20210301154101.ke5j2r3lazjlxrsl@pali/
> > >
> > > Anyway, I'm looking at differences between ultra and non-ultra boards
> > > which are used by U-Boot... And I see only:
> > > * switch configuration & phy
> > > * i2c rtc
> > > * sdhci slot
> > >
> > > Other changes are not used by U-Boot (led).
> > >
> > > For switch configuration & phy there is already custom code in U-Boot
> > > board file. For sdhci slot there is also (to enable/disable eMMC after
> > > detection).
> > >
> > > So the only difference is presence of i2c rtc, right?
> > >
> > > I guess it does not make much sense to copy and duplicate whole DTS and
> > > also defconfig file for such small differences. Insertion of just few
> > > nodes is not a big problem.
> > >
> > > These boars are not very different, and having tons of u-boot binaries
> > > for every combination just complicate it as explained in above email.
> >
> > I fully agree with Pali. It's much more conveniant to just have one
> > U-Boot target (and binary) which can handle multiple board variants
> > by runtime detection. I would very much welcome to see the support for
> > the "Ultra" variant added this way.
> 
> I understand your points and concerns.
> 
> I'm just worried that this counterproductive as I don't see the same thing
> happening in Linux and looking at this we could make similar arguments
> for other boards, which aren't so different from the ESPRESSOBin boards
> with the exception of the name.

I have already opened this question also in Linux, but there was no feedback:
https://lore.kernel.org/linux-devicetree/20201022140007.hppmeyt34lubotbc@pali/t/#u

Anyway, as U-Boot can detect variant of the board, it also can load
specific DTS kernel file for detected variant. So it has still advantage
even when Linux does not use it (y

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Fri, Aug 13, 2021 at 12:29 PM Pali Rohár  wrote:
>
> On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig 
> > b/board/Marvell/mvebu_armada-37xx/Kconfig
> > new file mode 100644
> > index 00..b84dd20023
> > --- /dev/null
> > +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> > @@ -0,0 +1,29 @@
> > +menu "Marvell Armada 37xx configuration"
> > +depends on TARGET_MVEBU_ARMADA_37XX
> > +
> > +config MVEBU_MAC_HW_INFO
> > + bool "Marvell hw_info (mac) support"
> > + depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> > + default n
> > + help
> > +   Enable loading of the Marvell hw_info parameters from the
> > +   SPI flash hw_info area. Parameters (usually the board serial
> > +   number and MAC addresses) are then imported into the
> > +   existing U-Boot environment.
> > +   Implementation of this command is compatible with the
> > +   original Marvell U-Boot command. Reading and writing is
> > +   supported.
> > +   EEPROM config pattern and checksum aren't supported.
> > +   After enabled, these parameters are managed from the common
> > +   U-Boot mac command.
> > +
> > +config MVEBU_MAC_HW_INFO_OFFSET
> > + hex "Marvell hw_info (mac) SPI flash offset"
> > + depends on MVEBU_MAC_HW_INFO
> > + default 0x3E
> > + help
> > +   This option defines the SPI flash offset of the Marvell
> > +   hw_info area. This defaults to 0x3E on most Armada
> > +   A3720 platforms.
>
> Just a question, cannot we load this offset from DTS? In DTS are already
> specified SPI partitions, so this could eliminate need for defining this
> offset at two places. But I really do not know at which time is this
> code called, if DTB is available at this time or not.

The code is called right after cpu_secondary_init_r.
I'm not sure, there also some other values, which are hard-coded and
so far I didn't really see any other possible offset.

Are you aware of any other relevant board with the Marvell hw_info
parameters?

>
> > +endmenu

Kind regards,
Luka

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Fri, Aug 13, 2021 at 12:22 PM Pali Rohár  wrote:
>
> On Friday 13 August 2021 12:03:57 Luka Kovacic wrote:
> > Hello Pali,
> >
> > On Fri, Aug 13, 2021 at 11:27 AM Pali Rohár  wrote:
> > >
> > > On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> > > > Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> > > > Technologies, Inc.
> > > >
> > > > The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> > > > Peripherals:
> > > >  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
> > > >  - RTC clock (PCF8563)
> > > >  - USB 3.0 port
> > > >  - USB 2.0 port
> > > >  - 4x LED
> > > >  - UART over Micro-USB
> > > >  - M.2 slot (2280)
> > > >  - Mini PCI-E slot
> > > >
> > > > Additionally, automatic import of the Marvell hw_info parameters is
> > > > enabled via the recently added mac command for A37XX platforms.
> > > > The parameters stored in Marvell hw_info are usually the board serial
> > > > number and MAC addresses.
> > > >
> > > > Signed-off-by: Luka Kovacic 
> > > > Cc: Luka Perkov 
> > > > Cc: Robert Marko 
> > > > ---
> > > >  arch/arm/dts/Makefile |   1 +
> > > >  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> > > >  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> > > >  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
> > > >  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> > > >  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> > > >  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> > > >  7 files changed, 514 insertions(+), 203 deletions(-)
> > > >  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> > > >  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> > > >  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> > > ...
> > > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c 
> > > > b/board/Marvell/mvebu_armada-37xx/board.c
> > > > index 2de9c2ac17..21c1eb7b22 100644
> > > > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > > > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > > > @@ -11,6 +11,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > @@ -55,6 +56,15 @@ DECLARE_GLOBAL_DATA_PTR;
> > > >  #define MVEBU_G2_SMI_PHY_CMD_REG (24)
> > > >  #define MVEBU_G2_SMI_PHY_DATA_REG(25)
> > > >
> > > > +/* Marvell 88E1512 */
> > > > +#define MII_MARVELL_PHY_PAGE 22
> > > > +
> > > > +#define MV88E1512_GENERAL_CTRL   20
> > > > +#define MV88E1512_MODE_SGMII 1
> > > > +#define MV88E1512_RESET_OFFS 15
> > > > +
> > > > +#define ULTRA_MV88E1512_PHYADDR  0x1
> > > > +
> > > >  /*
> > > >   * Memory Controller Registers
> > > >   *
> > > > @@ -282,12 +292,68 @@ static int mii_multi_chip_mode_write(struct 
> > > > mii_dev *bus, int dev_smi_addr,
> > > >   return 0;
> > > >  }
> > > >
> > > > -/* Bring-up board-specific network stuff */
> > > > -int board_network_enable(struct mii_dev *bus)
> > > > +void force_phy_88e1512_sgmii_to_copper(u16 devaddr)
> > > >  {
> > > > - if (!of_machine_is_compatible("globalscale,espressobin"))
> > > > - return 0;
> > > > + const char *name;
> > > > + u16 reg;
> > > > +
> > > > + name = miiphy_get_current_dev();
> > > > + if (name) {
> > >
> > > It is possible that phy is not available? As you are calling code below
> > > only in case name is not-NULL.
> >
> > Well, according to common/miiphyutil.c, it could also happen that it's NULL.
>
> I see. But if it happens, is not it fatal error for this board? If name
> is NULL then you cannot correctly configure board correctly, right?
>
> > >
> > > > + /* SGMII-to-Copper mode initialization */
> > > > +
> > > > + /* Select page 18 */
> > > > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0x12);
> > > > + /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > > > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> > > > + reg &= ~0x7;
> > > > + reg |= MV88E1512_MODE_SGMII;
> > > > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> > > > + /* PHY reset is necessary after changing MODE[2:0] */
> > > > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> > > > + reg |= 1 << MV88E1512_RESET_OFFS;
> > > > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> > > > + /* Reset page selection */
> > > > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0);
> > > > + udelay(100);
> > > > + }
> > > > +}
> > > > +
> > > > +int board_network_enable_espressobin_ultra(struct mii_dev *bus)
> > > > +{
> > > > + int i;
> > > > + /* Setup 88E1512 SGMII-to-Copper mode */
> > > > + force_phy_88e1512_s

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig 
> b/board/Marvell/mvebu_armada-37xx/Kconfig
> new file mode 100644
> index 00..b84dd20023
> --- /dev/null
> +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> @@ -0,0 +1,29 @@
> +menu "Marvell Armada 37xx configuration"
> +depends on TARGET_MVEBU_ARMADA_37XX
> +
> +config MVEBU_MAC_HW_INFO
> + bool "Marvell hw_info (mac) support"
> + depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> + default n
> + help
> +   Enable loading of the Marvell hw_info parameters from the
> +   SPI flash hw_info area. Parameters (usually the board serial
> +   number and MAC addresses) are then imported into the
> +   existing U-Boot environment.
> +   Implementation of this command is compatible with the
> +   original Marvell U-Boot command. Reading and writing is
> +   supported.
> +   EEPROM config pattern and checksum aren't supported.
> +   After enabled, these parameters are managed from the common
> +   U-Boot mac command.
> +
> +config MVEBU_MAC_HW_INFO_OFFSET
> + hex "Marvell hw_info (mac) SPI flash offset"
> + depends on MVEBU_MAC_HW_INFO
> + default 0x3E
> + help
> +   This option defines the SPI flash offset of the Marvell
> +   hw_info area. This defaults to 0x3E on most Armada
> +   A3720 platforms.

Just a question, cannot we load this offset from DTS? In DTS are already
specified SPI partitions, so this could eliminate need for defining this
offset at two places. But I really do not know at which time is this
code called, if DTB is available at this time or not.

> +endmenu

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

Hello Stefan and Pali,

On Fri, Aug 13, 2021 at 11:58 AM Stefan Roese  wrote:
>
> Hi,
>
> On 13.08.21 11:54, Pali Rohár wrote:
> > On Friday 13 August 2021 11:08:08 Luka Kovacic wrote:
> >> Hello Pali,
> >>
> >> On Fri, Aug 13, 2021 at 10:14 AM Pali Rohár  wrote:
> >>>
> >>> On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
>  Add initial support for the ESPRESSOBin-Ultra board from Globalscale
>  Technologies, Inc.
> 
>  The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
>  Peripherals:
>    - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
>    - RTC clock (PCF8563)
>    - USB 3.0 port
>    - USB 2.0 port
>    - 4x LED
>    - UART over Micro-USB
>    - M.2 slot (2280)
>    - Mini PCI-E slot
> 
>  Additionally, automatic import of the Marvell hw_info parameters is
>  enabled via the recently added mac command for A37XX platforms.
>  The parameters stored in Marvell hw_info are usually the board serial
>  number and MAC addresses.
> 
>  Signed-off-by: Luka Kovacic 
>  Cc: Luka Perkov 
>  Cc: Robert Marko 
>  ---
>    arch/arm/dts/Makefile |   1 +
>    .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
>    arch/arm/dts/armada-3720-espressobin.dts  | 199 +
>    arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
>    board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
>    board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
>    .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
>    7 files changed, 514 insertions(+), 203 deletions(-)
>    create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
>    create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>    create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> >>>
> >>> Hello Luka! Please look at my comments from previous review:
> >>> https://lore.kernel.org/u-boot/20210227004852.5urcwnn6uxehuk72@pali/
> >>>
> >>> I think it is not a good idea to duplicate espressobin code, specially
> >>> now when differences between v5, v7, non-emmc and emmc were
> >>> de-duplicated and correctly detected at runtime. Just use one DTS and
> >>> one config file and differences can be handled in board code functions
> >>> "board_fix_fdt" and "board_late_init".
> >>
> >> I believe that patching the DTS at runtime diminishes the value of
> >> device trees in the first case.
> >>
> >> As far as the v5, v7, non-emmc and emmc boards go I do understand
> >> that, as they are physically similar and more or less different revisions
> >> of the same board.
> >>
> >> The ESPRESSOBin Ultra board is completely different from those boards
> >> physically and so I doesn't make sense to me, why we would merge all
> >> of them into one device tree.
> >
> > See email for reasons:
> > https://lore.kernel.org/u-boot/20210301154101.ke5j2r3lazjlxrsl@pali/
> >
> > Anyway, I'm looking at differences between ultra and non-ultra boards
> > which are used by U-Boot... And I see only:
> > * switch configuration & phy
> > * i2c rtc
> > * sdhci slot
> >
> > Other changes are not used by U-Boot (led).
> >
> > For switch configuration & phy there is already custom code in U-Boot
> > board file. For sdhci slot there is also (to enable/disable eMMC after
> > detection).
> >
> > So the only difference is presence of i2c rtc, right?
> >
> > I guess it does not make much sense to copy and duplicate whole DTS and
> > also defconfig file for such small differences. Insertion of just few
> > nodes is not a big problem.
> >
> > These boars are not very different, and having tons of u-boot binaries
> > for every combination just complicate it as explained in above email.
>
> I fully agree with Pali. It's much more conveniant to just have one
> U-Boot target (and binary) which can handle multiple board variants
> by runtime detection. I would very much welcome to see the support for
> the "Ultra" variant added this way.

I understand your points and concerns.

I'm just worried that this counterproductive as I don't see the same thing
happening in Linux and looking at this we could make similar arguments
for other boards, which aren't so different from the ESPRESSOBin boards
with the exception of the name.

>
> > If you need help with this I can try to do something... as I was already
> > involved in unification of all v5/v7/emmc/non-emmc variants into one
> > binary with one DTS.
>
> Thanks Pali for all your work here.
>
> Thanks,
> Stefan
>
> >> I resorted to the Linux way and used a common dtsi for both the
> >> ESPRESSOBin - which is the base and the ESPRESSOBin-Ultra.
> >>
> >>>
>  diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>  index c42715ead4..f21c9c94d3 100644
>  --- a/arch/arm/dts/Makefile
>  +++ b/arch/arm/dts/Makefile
>  @@ -213,6 +213,7 @@ dtb-$(CONFIG_ARCH_TEGRA) +=

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Friday 13 August 2021 12:03:57 Luka Kovacic wrote:
> Hello Pali,
> 
> On Fri, Aug 13, 2021 at 11:27 AM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> > > Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> > > Technologies, Inc.
> > >
> > > The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> > > Peripherals:
> > >  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
> > >  - RTC clock (PCF8563)
> > >  - USB 3.0 port
> > >  - USB 2.0 port
> > >  - 4x LED
> > >  - UART over Micro-USB
> > >  - M.2 slot (2280)
> > >  - Mini PCI-E slot
> > >
> > > Additionally, automatic import of the Marvell hw_info parameters is
> > > enabled via the recently added mac command for A37XX platforms.
> > > The parameters stored in Marvell hw_info are usually the board serial
> > > number and MAC addresses.
> > >
> > > Signed-off-by: Luka Kovacic 
> > > Cc: Luka Perkov 
> > > Cc: Robert Marko 
> > > ---
> > >  arch/arm/dts/Makefile |   1 +
> > >  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> > >  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> > >  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
> > >  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> > >  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> > >  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> > >  7 files changed, 514 insertions(+), 203 deletions(-)
> > >  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> > >  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> > >  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> > ...
> > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c 
> > > b/board/Marvell/mvebu_armada-37xx/board.c
> > > index 2de9c2ac17..21c1eb7b22 100644
> > > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > > @@ -11,6 +11,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -55,6 +56,15 @@ DECLARE_GLOBAL_DATA_PTR;
> > >  #define MVEBU_G2_SMI_PHY_CMD_REG (24)
> > >  #define MVEBU_G2_SMI_PHY_DATA_REG(25)
> > >
> > > +/* Marvell 88E1512 */
> > > +#define MII_MARVELL_PHY_PAGE 22
> > > +
> > > +#define MV88E1512_GENERAL_CTRL   20
> > > +#define MV88E1512_MODE_SGMII 1
> > > +#define MV88E1512_RESET_OFFS 15
> > > +
> > > +#define ULTRA_MV88E1512_PHYADDR  0x1
> > > +
> > >  /*
> > >   * Memory Controller Registers
> > >   *
> > > @@ -282,12 +292,68 @@ static int mii_multi_chip_mode_write(struct mii_dev 
> > > *bus, int dev_smi_addr,
> > >   return 0;
> > >  }
> > >
> > > -/* Bring-up board-specific network stuff */
> > > -int board_network_enable(struct mii_dev *bus)
> > > +void force_phy_88e1512_sgmii_to_copper(u16 devaddr)
> > >  {
> > > - if (!of_machine_is_compatible("globalscale,espressobin"))
> > > - return 0;
> > > + const char *name;
> > > + u16 reg;
> > > +
> > > + name = miiphy_get_current_dev();
> > > + if (name) {
> >
> > It is possible that phy is not available? As you are calling code below
> > only in case name is not-NULL.
> 
> Well, according to common/miiphyutil.c, it could also happen that it's NULL.

I see. But if it happens, is not it fatal error for this board? If name
is NULL then you cannot correctly configure board correctly, right?

> >
> > > + /* SGMII-to-Copper mode initialization */
> > > +
> > > + /* Select page 18 */
> > > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0x12);
> > > + /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> > > + reg &= ~0x7;
> > > + reg |= MV88E1512_MODE_SGMII;
> > > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> > > + /* PHY reset is necessary after changing MODE[2:0] */
> > > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> > > + reg |= 1 << MV88E1512_RESET_OFFS;
> > > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> > > + /* Reset page selection */
> > > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0);
> > > + udelay(100);
> > > + }
> > > +}
> > > +
> > > +int board_network_enable_espressobin_ultra(struct mii_dev *bus)
> > > +{
> > > + int i;
> > > + /* Setup 88E1512 SGMII-to-Copper mode */
> > > + force_phy_88e1512_sgmii_to_copper(ULTRA_MV88E1512_PHYADDR);
> > >
> > > + /*
> > > +  * FIXME: remove this code once Topaz driver gets available
> > > +  * A3720 ESPRESSObin Ultra Board Only
> > > +  * Configure Topaz switch (88E6341)
> > > +  * Set port 1,2,3,4,5 to forwarding Mode (thro

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Fri, Aug 13, 2021 at 12:09 PM Pali Rohár  wrote:
>
> On Friday 13 August 2021 11:51:02 Luka Kovacic wrote:
> > Hello Pali,
> >
> > On Fri, Aug 13, 2021 at 11:41 AM Pali Rohár  wrote:
> > >
> > > On Friday 13 August 2021 11:23:15 Luka Kovacic wrote:
> > > > Hello Marek and Pali,
> > > >
> > > > On Fri, Aug 13, 2021 at 10:23 AM Pali Rohár  wrote:
> > > > >
> > > > > On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > > > > > The mac command is implemented to enable parsing Marvell hw_info 
> > > > > > formatted
> > > > > > environments. This format is often used on Marvell Armada A37XX 
> > > > > > based
> > > > > > devices to store parameters like the board serial number, factory
> > > > > > MAC addresses and some other information.
> > > > > > These parameters are usually written to the flash in the factory.
> > > > > >
> > > > > > Currently the mac command supports reading/writing parameters and 
> > > > > > dumping
> > > > > > the current hw_info parameters.
> > > > > > EEPROM config pattern and checksum aren't supported.
> > > > > >
> > > > > > This functionality has been tested on the GST ESPRESSOBin-Ultra 
> > > > > > board
> > > > > > successfully, both reading the stock U-Boot parameters in mainline 
> > > > > > U-Boot
> > > > > > and reading the parameters written by this command in the stock 
> > > > > > U-Boot.
> > > > > >
> > > > > > Usage example:
> > > > > >  => mac read
> > > > > >  => saveenv
> > > > >
> > > > > Hello Luka!
> > > > >
> > > > > You are just using above commands for accessing following variables,
> > > > > right?
> > > > >
> > > > > pcb_slm
> > > > > pcb_rev
> > > > > eco_rev
> > > > > pcb_sn
> > > > > ethaddr
> > > > > eth1addr
> > > > > eth2addr
> > > > > eth3addr
> > > > > eth4addr
> > > > > eth5addr
> > > > > eth6addr
> > > > > eth7addr
> > > > > eth8addr
> > > > > eth9addr
> > > > >
> > > >
> > > > Yes, I am accessing these variables.
> > > >
> > > > > So why is additional command required? Espressobin boards can already
> > > > > load and use correct eth*addr variables, which is implemented in board
> > > > > file.
> > > >
> > > > The current implementation in the board file preserves currently stored 
> > > > MAC
> > > > addresses, which are in the U-Boot environment.
> > >
> > > Yes, this is for v5/v7 variants. But it can be easily modified for ultra
> > > to read mac addresses from this custom storage.
> > >
> > > > My implementation is suitable for the ESPRESSOBin-Ultra, as the MACs
> > > > are stored in the hw_info area of the SPI flash and the tool imports 
> > > > them
> > > > into the normal U-Boot env.
> > > > They can also be modified using the tool - we won't be able to get this
> > > > functionality in the board file.
> > >
> > > Modification by env variables is also possible from the board file. Look
> > > at U_BOOT_ENV_CALLBACK() functionality, which allows to call custom code
> > > when trying to change/delete particular env variable.
> > >
> > > > > So what about implementing this import for all above variables for 
> > > > > ultra
> > > > > variant in board file, like it is already for all other variants? I 
> > > > > have
> > > > > already suggested it in the past. As I think this approach is just one
> > > > > giant and complicated hack...
> > > > > https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/
> > > > >
> > > > > I guess that it should be enough to import variables from SPI to env 
> > > > > in
> > > > > board_late_init() function.
> > > >
> > > > Currently, the MACs are automatically imported from the
> > > > common/board_r.c file at boot, I think this is more standard and 
> > > > cleaner.
> > >
> > > Hm... then I somehow misunderstood how it works.
> > >
> > > Because I do not see any change in common/board_r.c which could read or
> > > import mac addresses via calling this new "mac read" command.
> >
> > The function mac_read_from_eeprom() is called, if CONFIG_ID_EEPROM is
> > enabled. This has also been tested and a mechanism was added to avoid
> > duplicate imports (if the user wants to modify the MAC address, so it then
> > isn't overwritten).
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/common/board_r.c#L724
>
> Ok, thank you. Now I see how it works. From commit message and from the
> first look at the code it was not clear for me.
>
> So in this patch you are implementing:
>
> 1) mac_read_from_eeprom() function for CONFIG_ID_EEPROM
>
> 2) do_mac() function for CONFIG_ID_EEPROM
>
> which looks fine now.
>
> > >
> > > > Nevertheless, I believe this a sufficient solution for now,
> > > > as this has been suggested as a solution in the previous patchset
> > > > comments too.
>
> Ok, this seems to be sufficient.
>
> > > > >
> > > > > > Signed-off-by: Luka Kovacic 
> > > > > > Cc: Luka Perkov 
> > > > > > Cc: Robert Marko 
> > > > > > ---
> > > > > >  arch/arm/mach-mvebu/Kconfig   |   1 +
> > > > > >  board/Marvell/mvebu_armada-37xx/Kconfig   |  29 ++
> > > > > >  board/Marvell/mvebu

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Friday 13 August 2021 11:51:02 Luka Kovacic wrote:
> Hello Pali,
> 
> On Fri, Aug 13, 2021 at 11:41 AM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 11:23:15 Luka Kovacic wrote:
> > > Hello Marek and Pali,
> > >
> > > On Fri, Aug 13, 2021 at 10:23 AM Pali Rohár  wrote:
> > > >
> > > > On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > > > > The mac command is implemented to enable parsing Marvell hw_info 
> > > > > formatted
> > > > > environments. This format is often used on Marvell Armada A37XX based
> > > > > devices to store parameters like the board serial number, factory
> > > > > MAC addresses and some other information.
> > > > > These parameters are usually written to the flash in the factory.
> > > > >
> > > > > Currently the mac command supports reading/writing parameters and 
> > > > > dumping
> > > > > the current hw_info parameters.
> > > > > EEPROM config pattern and checksum aren't supported.
> > > > >
> > > > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > > > successfully, both reading the stock U-Boot parameters in mainline 
> > > > > U-Boot
> > > > > and reading the parameters written by this command in the stock 
> > > > > U-Boot.
> > > > >
> > > > > Usage example:
> > > > >  => mac read
> > > > >  => saveenv
> > > >
> > > > Hello Luka!
> > > >
> > > > You are just using above commands for accessing following variables,
> > > > right?
> > > >
> > > > pcb_slm
> > > > pcb_rev
> > > > eco_rev
> > > > pcb_sn
> > > > ethaddr
> > > > eth1addr
> > > > eth2addr
> > > > eth3addr
> > > > eth4addr
> > > > eth5addr
> > > > eth6addr
> > > > eth7addr
> > > > eth8addr
> > > > eth9addr
> > > >
> > >
> > > Yes, I am accessing these variables.
> > >
> > > > So why is additional command required? Espressobin boards can already
> > > > load and use correct eth*addr variables, which is implemented in board
> > > > file.
> > >
> > > The current implementation in the board file preserves currently stored 
> > > MAC
> > > addresses, which are in the U-Boot environment.
> >
> > Yes, this is for v5/v7 variants. But it can be easily modified for ultra
> > to read mac addresses from this custom storage.
> >
> > > My implementation is suitable for the ESPRESSOBin-Ultra, as the MACs
> > > are stored in the hw_info area of the SPI flash and the tool imports them
> > > into the normal U-Boot env.
> > > They can also be modified using the tool - we won't be able to get this
> > > functionality in the board file.
> >
> > Modification by env variables is also possible from the board file. Look
> > at U_BOOT_ENV_CALLBACK() functionality, which allows to call custom code
> > when trying to change/delete particular env variable.
> >
> > > > So what about implementing this import for all above variables for ultra
> > > > variant in board file, like it is already for all other variants? I have
> > > > already suggested it in the past. As I think this approach is just one
> > > > giant and complicated hack...
> > > > https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/
> > > >
> > > > I guess that it should be enough to import variables from SPI to env in
> > > > board_late_init() function.
> > >
> > > Currently, the MACs are automatically imported from the
> > > common/board_r.c file at boot, I think this is more standard and cleaner.
> >
> > Hm... then I somehow misunderstood how it works.
> >
> > Because I do not see any change in common/board_r.c which could read or
> > import mac addresses via calling this new "mac read" command.
> 
> The function mac_read_from_eeprom() is called, if CONFIG_ID_EEPROM is
> enabled. This has also been tested and a mechanism was added to avoid
> duplicate imports (if the user wants to modify the MAC address, so it then
> isn't overwritten).
> 
> https://source.denx.de/u-boot/u-boot/-/blob/master/common/board_r.c#L724

Ok, thank you. Now I see how it works. From commit message and from the
first look at the code it was not clear for me.

So in this patch you are implementing:

1) mac_read_from_eeprom() function for CONFIG_ID_EEPROM

2) do_mac() function for CONFIG_ID_EEPROM

which looks fine now.

> >
> > > Nevertheless, I believe this a sufficient solution for now,
> > > as this has been suggested as a solution in the previous patchset
> > > comments too.

Ok, this seems to be sufficient.

> > > >
> > > > > Signed-off-by: Luka Kovacic 
> > > > > Cc: Luka Perkov 
> > > > > Cc: Robert Marko 
> > > > > ---
> > > > >  arch/arm/mach-mvebu/Kconfig   |   1 +
> > > > >  board/Marvell/mvebu_armada-37xx/Kconfig   |  29 ++
> > > > >  board/Marvell/mvebu_armada-37xx/Makefile  |   3 +-
> > > > >  board/Marvell/mvebu_armada-37xx/mac/Makefile  |   5 +
> > > > >  board/Marvell/mvebu_armada-37xx/mac/hw_info.c | 391 
> > > > > ++

What about renaming this file just to?

board/Marvell/mvebu_armada-37xx/mac.c

I do not know if we need separate directory just for one hw_info.c file.

> > > > >  include/configs

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

Hello Pali,

On Fri, Aug 13, 2021 at 11:27 AM Pali Rohár  wrote:
>
> On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> > Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> > Technologies, Inc.
> >
> > The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> > Peripherals:
> >  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
> >  - RTC clock (PCF8563)
> >  - USB 3.0 port
> >  - USB 2.0 port
> >  - 4x LED
> >  - UART over Micro-USB
> >  - M.2 slot (2280)
> >  - Mini PCI-E slot
> >
> > Additionally, automatic import of the Marvell hw_info parameters is
> > enabled via the recently added mac command for A37XX platforms.
> > The parameters stored in Marvell hw_info are usually the board serial
> > number and MAC addresses.
> >
> > Signed-off-by: Luka Kovacic 
> > Cc: Luka Perkov 
> > Cc: Robert Marko 
> > ---
> >  arch/arm/dts/Makefile |   1 +
> >  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> >  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> >  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
> >  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> >  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> >  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> >  7 files changed, 514 insertions(+), 203 deletions(-)
> >  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> >  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> >  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> ...
> > diff --git a/board/Marvell/mvebu_armada-37xx/board.c 
> > b/board/Marvell/mvebu_armada-37xx/board.c
> > index 2de9c2ac17..21c1eb7b22 100644
> > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > @@ -11,6 +11,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -55,6 +56,15 @@ DECLARE_GLOBAL_DATA_PTR;
> >  #define MVEBU_G2_SMI_PHY_CMD_REG (24)
> >  #define MVEBU_G2_SMI_PHY_DATA_REG(25)
> >
> > +/* Marvell 88E1512 */
> > +#define MII_MARVELL_PHY_PAGE 22
> > +
> > +#define MV88E1512_GENERAL_CTRL   20
> > +#define MV88E1512_MODE_SGMII 1
> > +#define MV88E1512_RESET_OFFS 15
> > +
> > +#define ULTRA_MV88E1512_PHYADDR  0x1
> > +
> >  /*
> >   * Memory Controller Registers
> >   *
> > @@ -282,12 +292,68 @@ static int mii_multi_chip_mode_write(struct mii_dev 
> > *bus, int dev_smi_addr,
> >   return 0;
> >  }
> >
> > -/* Bring-up board-specific network stuff */
> > -int board_network_enable(struct mii_dev *bus)
> > +void force_phy_88e1512_sgmii_to_copper(u16 devaddr)
> >  {
> > - if (!of_machine_is_compatible("globalscale,espressobin"))
> > - return 0;
> > + const char *name;
> > + u16 reg;
> > +
> > + name = miiphy_get_current_dev();
> > + if (name) {
>
> It is possible that phy is not available? As you are calling code below
> only in case name is not-NULL.

Well, according to common/miiphyutil.c, it could also happen that it's NULL.

>
> > + /* SGMII-to-Copper mode initialization */
> > +
> > + /* Select page 18 */
> > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0x12);
> > + /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> > + reg &= ~0x7;
> > + reg |= MV88E1512_MODE_SGMII;
> > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> > + /* PHY reset is necessary after changing MODE[2:0] */
> > + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> > + reg |= 1 << MV88E1512_RESET_OFFS;
> > + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> > + /* Reset page selection */
> > + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0);
> > + udelay(100);
> > + }
> > +}
> > +
> > +int board_network_enable_espressobin_ultra(struct mii_dev *bus)
> > +{
> > + int i;
> > + /* Setup 88E1512 SGMII-to-Copper mode */
> > + force_phy_88e1512_sgmii_to_copper(ULTRA_MV88E1512_PHYADDR);
> >
> > + /*
> > +  * FIXME: remove this code once Topaz driver gets available
> > +  * A3720 ESPRESSObin Ultra Board Only
> > +  * Configure Topaz switch (88E6341)
> > +  * Set port 1,2,3,4,5 to forwarding Mode (through Switch Port 
> > registers)
> > +  */
> > + for (i = 0; i <= 5; i++) {
> > + mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(i),
> > +   MVEBU_SW_PORT_CTRL_REG,
> > +   i == 5 ? 0x7c : 0x7f);
>
> Why port 5 has different settings?

It's to disable forwarding between the WAN port and LAN ports (I've
tested this).
I'm aware of thi

Re: Hints on how to use efi_driver/efi_block_device.c

2021-08-13 Thread Heinrich Schuchardt

On 8/13/21 9:56 AM, Christian Melki wrote:

On 8/13/21 2:36 AM, Heinrich Schuchardt wrote:

On 8/12/21 11:49 PM, Simon Glass wrote:

+Heinrich Schuchardt too

On Thu, 12 Aug 2021 at 08:35, Christian Melki
wrote:

I was hoping that U-boot would detect BLOCK_IO devices provided by UEFI
automatically. But I can't see anything attached under lsblk or find
some other information about this.
For example, Barebox works just fine on both Virtualbox and real
hardware in this regard.

Barebox does not have an emmc driver for the real hardware but
piggybacks off the UEFI protocol for this.

Hello Christian,

U-Boot can be used in two scenarios:

1) U-Boot is the firmware providing the UEFI API
2) U-Boot is running as an application consuming the UEFI API.

Barebox only supports scenario 2).

Ok.

I'm sure that the idea with this U-boot driver is something like that,
but would appreciate some hints on how to use it.

efi_driver/efi_block_device.c is used in scenario 1).

When a UEFI payload like iPXE provides an EFI_BLOCK_IO_PROTOCOL on a
handle and calls ConnectController() U-Boot will install the
EFI_SIMPLE_FILE_PROTOCOL for each partition on the block device.

You can find a detailed description of this use case in:

* https://u-boot.readthedocs.io/en/latest/develop/uefi/iscsi.html
* https://archive.fosdem.org/2020/schedule/event/firmware_duwu/

I read the fosdem presentation, but apparently did not understand it
correctly, as I thought it could be used for presenting the UEFI block
IO protocol.

Ie, standard usage. UEFI boots of a drive, posts some handles(?) to a
block device and U-boot picks it up, not knowing more about the
abstracted hardware.

Here you seem be referring to scenario 2).

For scenario 2) support for UEFI block devices has not been implemented,
yet. As operating systems like Linux, BSD, Windows all can be booted via
UEFI there has not been any use case driving further development of this
scenario.

Please, describe what you want to do with U-Boot.

I have an x86 board (DFI GHF51, AMD Ryzen R1000) with an ACPI presented
eMMC. AMDI0040. U-boot does not seem to recognize this device (Linux
works fine.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/host/sdhci-acpi.c).

So I thought the UEFI block IO protocol could come in handy as a generic
abstraction. The device works fine under Barebox with this mechanism. So
without a native or an UEFI block IO driver I have no disc access in
U-boot.

I know that UEFI can boot a EFI-stubbed kernel directly. But I am more
comfortable with the U-boot "ecosystem" than UEFI, also it is easier to
merge more platforms under the same (existing) boot
mechanism/behavior/environment using U-boot as an intermediate than just
UEFI (older ARM/PPC etc).

There are two ways U-Boot can run on U-Boot:

CONFIG_EFI_STUB, efi-x86_payload64_defconfig: efi_main() calls
ExitBootServices(). After this point U-Boot cannot consume any EFI boot
services and protocols but has to rely on its own drivers.

CONFIG_EFI_APP, efi-x86_app_defconfig: U-Boot drivers use the UEFI API.
Currently the only drivers implemented in this way are:

- console: drivers/serial/serial_efi.c, "efi,uart"
- system reset: lib/efi/efi_app.c, "efi,reset"
- video: drivers/video/efi.c, "efi-fb"

drivers/video/efi.c is assuming that you have a VESA compatible graphics
card and is bypassing the UEFI API to access the framebuffer. To be
portable it would have been preferable to provide a UCLASS_VIDEO_CONSOLE
driver based on the graphical output protocol.

setup_memory() is another function that is not portable because it tries
to access global_data_ptr directly. This variable only exists on x86_64.
On ARM and RISC-V set_gd() should be used.

Best regards

Heinrich

So definitely an use-case for me.

Thanks in advance,
Christian

Best regards

Heinrich

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

2021-08-13 Thread Stefan Roese


Hi,

On 13.08.21 11:54, Pali Rohár wrote:

On Friday 13 August 2021 11:08:08 Luka Kovacic wrote:

Hello Pali,

On Fri, Aug 13, 2021 at 10:14 AM Pali Rohár  wrote:


On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:

Add initial support for the ESPRESSOBin-Ultra board from Globalscale
Technologies, Inc.

The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
Peripherals:
  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
  - RTC clock (PCF8563)
  - USB 3.0 port
  - USB 2.0 port
  - 4x LED
  - UART over Micro-USB
  - M.2 slot (2280)
  - Mini PCI-E slot

Additionally, automatic import of the Marvell hw_info parameters is
enabled via the recently added mac command for A37XX platforms.
The parameters stored in Marvell hw_info are usually the board serial
number and MAC addresses.

Signed-off-by: Luka Kovacic 
Cc: Luka Perkov 
Cc: Robert Marko 
---
  arch/arm/dts/Makefile |   1 +
  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
  7 files changed, 514 insertions(+), 203 deletions(-)
  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig


Hello Luka! Please look at my comments from previous review:
https://lore.kernel.org/u-boot/20210227004852.5urcwnn6uxehuk72@pali/

I think it is not a good idea to duplicate espressobin code, specially
now when differences between v5, v7, non-emmc and emmc were
de-duplicated and correctly detected at runtime. Just use one DTS and
one config file and differences can be handled in board code functions
"board_fix_fdt" and "board_late_init".


I believe that patching the DTS at runtime diminishes the value of
device trees in the first case.

As far as the v5, v7, non-emmc and emmc boards go I do understand
that, as they are physically similar and more or less different revisions
of the same board.

The ESPRESSOBin Ultra board is completely different from those boards
physically and so I doesn't make sense to me, why we would merge all
of them into one device tree.


See email for reasons:
https://lore.kernel.org/u-boot/20210301154101.ke5j2r3lazjlxrsl@pali/

Anyway, I'm looking at differences between ultra and non-ultra boards
which are used by U-Boot... And I see only:
* switch configuration & phy
* i2c rtc
* sdhci slot

Other changes are not used by U-Boot (led).

For switch configuration & phy there is already custom code in U-Boot
board file. For sdhci slot there is also (to enable/disable eMMC after
detection).

So the only difference is presence of i2c rtc, right?

I guess it does not make much sense to copy and duplicate whole DTS and
also defconfig file for such small differences. Insertion of just few
nodes is not a big problem.

These boars are not very different, and having tons of u-boot binaries
for every combination just complicate it as explained in above email.


I fully agree with Pali. It's much more conveniant to just have one
U-Boot target (and binary) which can handle multiple board variants
by runtime detection. I would very much welcome to see the support for
the "Ultra" variant added this way.


If you need help with this I can try to do something... as I was already
involved in unification of all v5/v7/emmc/non-emmc variants into one
binary with one DTS.


Thanks Pali for all your work here.

Thanks,
Stefan


I resorted to the Linux way and used a common dtsi for both the
ESPRESSOBin - which is the base and the ESPRESSOBin-Ultra.




diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index c42715ead4..f21c9c94d3 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -213,6 +213,7 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
  dtb-$(CONFIG_ARCH_MVEBU) +=  \
   armada-3720-db.dtb  \
   armada-3720-espressobin.dtb \
+ armada-3720-espressobin-ultra.dtb   \
   armada-3720-turris-mox.dtb  \
   armada-3720-uDPU.dtb\
   armada-375-db.dtb   \
diff --git a/arch/arm/dts/armada-3720-espressobin-ultra.dts 
b/arch/arm/dts/armada-3720-espressobin-ultra.dts
new file mode 100644
index 00..5ad0c723e3
--- /dev/null
+++ b/arch/arm/dts/armada-3720-espressobin-ultra.dts
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree file for ESPRESSObin-Ultra board
+ * Copyright (C) 2016 Marvell
+ * Copyright (C) 2019 Globalscale technologies, Inc.
+ * Copyright (C) 2021 Sartura Ltd.
+ *
+ * Author: Jason Hung 
+ * Author: Luka Kovacic 
+ * Author: Vladimir

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Friday 13 August 2021 11:08:08 Luka Kovacic wrote:
> Hello Pali,
> 
> On Fri, Aug 13, 2021 at 10:14 AM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> > > Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> > > Technologies, Inc.
> > >
> > > The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> > > Peripherals:
> > >  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
> > >  - RTC clock (PCF8563)
> > >  - USB 3.0 port
> > >  - USB 2.0 port
> > >  - 4x LED
> > >  - UART over Micro-USB
> > >  - M.2 slot (2280)
> > >  - Mini PCI-E slot
> > >
> > > Additionally, automatic import of the Marvell hw_info parameters is
> > > enabled via the recently added mac command for A37XX platforms.
> > > The parameters stored in Marvell hw_info are usually the board serial
> > > number and MAC addresses.
> > >
> > > Signed-off-by: Luka Kovacic 
> > > Cc: Luka Perkov 
> > > Cc: Robert Marko 
> > > ---
> > >  arch/arm/dts/Makefile |   1 +
> > >  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> > >  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> > >  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
> > >  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> > >  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> > >  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> > >  7 files changed, 514 insertions(+), 203 deletions(-)
> > >  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> > >  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> > >  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
> >
> > Hello Luka! Please look at my comments from previous review:
> > https://lore.kernel.org/u-boot/20210227004852.5urcwnn6uxehuk72@pali/
> >
> > I think it is not a good idea to duplicate espressobin code, specially
> > now when differences between v5, v7, non-emmc and emmc were
> > de-duplicated and correctly detected at runtime. Just use one DTS and
> > one config file and differences can be handled in board code functions
> > "board_fix_fdt" and "board_late_init".
> 
> I believe that patching the DTS at runtime diminishes the value of
> device trees in the first case.
> 
> As far as the v5, v7, non-emmc and emmc boards go I do understand
> that, as they are physically similar and more or less different revisions
> of the same board.
> 
> The ESPRESSOBin Ultra board is completely different from those boards
> physically and so I doesn't make sense to me, why we would merge all
> of them into one device tree.

See email for reasons:
https://lore.kernel.org/u-boot/20210301154101.ke5j2r3lazjlxrsl@pali/

Anyway, I'm looking at differences between ultra and non-ultra boards
which are used by U-Boot... And I see only:
* switch configuration & phy
* i2c rtc
* sdhci slot

Other changes are not used by U-Boot (led).

For switch configuration & phy there is already custom code in U-Boot
board file. For sdhci slot there is also (to enable/disable eMMC after
detection).

So the only difference is presence of i2c rtc, right?

I guess it does not make much sense to copy and duplicate whole DTS and
also defconfig file for such small differences. Insertion of just few
nodes is not a big problem.

These boars are not very different, and having tons of u-boot binaries
for every combination just complicate it as explained in above email.

If you need help with this I can try to do something... as I was already
involved in unification of all v5/v7/emmc/non-emmc variants into one
binary with one DTS.

> I resorted to the Linux way and used a common dtsi for both the
> ESPRESSOBin - which is the base and the ESPRESSOBin-Ultra.
> 
> >
> > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > index c42715ead4..f21c9c94d3 100644
> > > --- a/arch/arm/dts/Makefile
> > > +++ b/arch/arm/dts/Makefile
> > > @@ -213,6 +213,7 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
> > >  dtb-$(CONFIG_ARCH_MVEBU) +=  \
> > >   armada-3720-db.dtb  \
> > >   armada-3720-espressobin.dtb \
> > > + armada-3720-espressobin-ultra.dtb   \
> > >   armada-3720-turris-mox.dtb  \
> > >   armada-3720-uDPU.dtb\
> > >   armada-375-db.dtb   \
> > > diff --git a/arch/arm/dts/armada-3720-espressobin-ultra.dts 
> > > b/arch/arm/dts/armada-3720-espressobin-ultra.dts
> > > new file mode 100644
> > > index 00..5ad0c723e3
> > > --- /dev/null
> > > +++ b/arch/arm/dts/armada-3720-espressobin-ultra.dts
> > > @@ -0,0 +1,114 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +/*
> > > + * Device Tree file for ESPRESSObin-Ultra board
> > > + * Copyright (C) 2016 Marvell
> > > + * Copyright (C) 2019 Globalscale technologies, Inc.
> > > + * Copyright (C) 2021 Sartura Ltd.
> > > + *
> > > + * Aut

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

Hello Pali,

On Fri, Aug 13, 2021 at 11:41 AM Pali Rohár  wrote:
>
> On Friday 13 August 2021 11:23:15 Luka Kovacic wrote:
> > Hello Marek and Pali,
> >
> > On Fri, Aug 13, 2021 at 10:23 AM Pali Rohár  wrote:
> > >
> > > On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > > > The mac command is implemented to enable parsing Marvell hw_info 
> > > > formatted
> > > > environments. This format is often used on Marvell Armada A37XX based
> > > > devices to store parameters like the board serial number, factory
> > > > MAC addresses and some other information.
> > > > These parameters are usually written to the flash in the factory.
> > > >
> > > > Currently the mac command supports reading/writing parameters and 
> > > > dumping
> > > > the current hw_info parameters.
> > > > EEPROM config pattern and checksum aren't supported.
> > > >
> > > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > > successfully, both reading the stock U-Boot parameters in mainline 
> > > > U-Boot
> > > > and reading the parameters written by this command in the stock U-Boot.
> > > >
> > > > Usage example:
> > > >  => mac read
> > > >  => saveenv
> > >
> > > Hello Luka!
> > >
> > > You are just using above commands for accessing following variables,
> > > right?
> > >
> > > pcb_slm
> > > pcb_rev
> > > eco_rev
> > > pcb_sn
> > > ethaddr
> > > eth1addr
> > > eth2addr
> > > eth3addr
> > > eth4addr
> > > eth5addr
> > > eth6addr
> > > eth7addr
> > > eth8addr
> > > eth9addr
> > >
> >
> > Yes, I am accessing these variables.
> >
> > > So why is additional command required? Espressobin boards can already
> > > load and use correct eth*addr variables, which is implemented in board
> > > file.
> >
> > The current implementation in the board file preserves currently stored MAC
> > addresses, which are in the U-Boot environment.
>
> Yes, this is for v5/v7 variants. But it can be easily modified for ultra
> to read mac addresses from this custom storage.
>
> > My implementation is suitable for the ESPRESSOBin-Ultra, as the MACs
> > are stored in the hw_info area of the SPI flash and the tool imports them
> > into the normal U-Boot env.
> > They can also be modified using the tool - we won't be able to get this
> > functionality in the board file.
>
> Modification by env variables is also possible from the board file. Look
> at U_BOOT_ENV_CALLBACK() functionality, which allows to call custom code
> when trying to change/delete particular env variable.
>
> > > So what about implementing this import for all above variables for ultra
> > > variant in board file, like it is already for all other variants? I have
> > > already suggested it in the past. As I think this approach is just one
> > > giant and complicated hack...
> > > https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/
> > >
> > > I guess that it should be enough to import variables from SPI to env in
> > > board_late_init() function.
> >
> > Currently, the MACs are automatically imported from the
> > common/board_r.c file at boot, I think this is more standard and cleaner.
>
> Hm... then I somehow misunderstood how it works.
>
> Because I do not see any change in common/board_r.c which could read or
> import mac addresses via calling this new "mac read" command.

The function mac_read_from_eeprom() is called, if CONFIG_ID_EEPROM is
enabled. This has also been tested and a mechanism was added to avoid
duplicate imports (if the user wants to modify the MAC address, so it then
isn't overwritten).

https://source.denx.de/u-boot/u-boot/-/blob/master/common/board_r.c#L724

>
> > Nevertheless, I believe this a sufficient solution for now,
> > as this has been suggested as a solution in the previous patchset
> > comments too.
> >
> > >
> > > > Signed-off-by: Luka Kovacic 
> > > > Cc: Luka Perkov 
> > > > Cc: Robert Marko 
> > > > ---
> > > >  arch/arm/mach-mvebu/Kconfig   |   1 +
> > > >  board/Marvell/mvebu_armada-37xx/Kconfig   |  29 ++
> > > >  board/Marvell/mvebu_armada-37xx/Makefile  |   3 +-
> > > >  board/Marvell/mvebu_armada-37xx/mac/Makefile  |   5 +
> > > >  board/Marvell/mvebu_armada-37xx/mac/hw_info.c | 391 ++
> > > >  include/configs/mvebu_armada-37xx.h   |   7 +
> > > >  lib/hashtable.c   |   2 +-
> > > >  7 files changed, 436 insertions(+), 2 deletions(-)
> > > >  create mode 100644 board/Marvell/mvebu_armada-37xx/Kconfig
> > > >  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/Makefile
> > > >  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> > > >
> > > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > > > index 89737a37ad..dff9f05b28 100644
> > > > --- a/arch/arm/mach-mvebu/Kconfig
> > > > +++ b/arch/arm/mach-mvebu/Kconfig
> > > > @@ -312,6 +312,7 @@ config SECURED_MODE_CSK_INDEX
> > > >   default 0
> > > >   depends on SECURED_MODE_IMAGE
> > > >
> > > > +source "board/Marvell/mvebu_armada-37xx/

Re: [PATCH v3 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

On Friday 13 August 2021 11:33:58 Luka Kovacic wrote:
> Hello Pali,
> 
> On Fri, Aug 13, 2021 at 11:31 AM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 10:59:33 Luka Kovacic wrote:
> > > Hello Pali,
> > >
> > > On Fri, Aug 13, 2021 at 10:08 AM Pali Rohár  wrote:
> > > >
> > > > On Friday 13 August 2021 01:39:37 Luka Kovacic wrote:
> > > > > Add the loadaddr U-Boot environment variable, as this is available in
> > > > > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
> > > >
> > > > Hello Luka! Why is this change needed? Reason that it is in historic
> > > > vendor U-Boot does not mean that it has to be also in new mainline
> > > > version.
> > > >
> > > > I have already wrote some reasons in previous review thread:
> > > > https://lore.kernel.org/u-boot/20210301150505.3iajeeufi7ahsnk3@pali/
> > > >
> > > > I think this change was needed when CONFIG_SYS_LOAD_ADDR was set to
> > > > incorrect value, which is already fixed in mainline U-Boot.
> > >
> > > This value is very useful when building custom Linux boot scripts.
> > > Yesterday, I booted the board without this patch and there was no loadaddr
> > > variable.
> > >
> > > Do I understand this correctly? Are you saying that the value in the 
> > > loadaddr
> > > variable should be automatically inherited from CONFIG_SYS_LOAD_ADDR
> > > even without this patch?
> >
> > If you do not specify load address then address from
> > CONFIG_SYS_LOAD_ADDR is used. But as I mentioned in above email,
> > CONFIG_SYS_LOAD_ADDR was defined incorrectly and which cause that it did
> > not worked and caused crashes...
> 
> I tried not specifying it and then the loadaddr variable doesn't even exist.

Could you send full example of your command?

> >
> > > >
> > > > > Signed-off-by: Luka Kovacic 
> > > > > Cc: Luka Perkov 
> > > > > Cc: Robert Marko 
> > > > > ---
> > > > >  include/configs/mvebu_armada-37xx.h | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/include/configs/mvebu_armada-37xx.h 
> > > > > b/include/configs/mvebu_armada-37xx.h
> > > > > index 8e8bcfa018..6901680e32 100644
> > > > > --- a/include/configs/mvebu_armada-37xx.h
> > > > > +++ b/include/configs/mvebu_armada-37xx.h
> > > > > @@ -110,6 +110,7 @@
> > > > >
> > > > >  /* fdt_addr and kernel_addr are needed for existing distribution 
> > > > > boot scripts */
> > > > >  #define CONFIG_EXTRA_ENV_SETTINGS\
> > > > > + "loadaddr=0x600\0"  \
> > > > >   "scriptaddr=0x6d0\0"\
> > > > >   "pxefile_addr_r=0x6e0\0"\
> > > > >   "fdt_addr=0x6f0\0"  \
> > > > > --
> > > > > 2.31.1
> > > > >
> > >
> > > Kind regards,
> > > Luka
> 
> Kind regards,
> Luka

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Friday 13 August 2021 11:23:15 Luka Kovacic wrote:
> Hello Marek and Pali,
> 
> On Fri, Aug 13, 2021 at 10:23 AM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > > The mac command is implemented to enable parsing Marvell hw_info formatted
> > > environments. This format is often used on Marvell Armada A37XX based
> > > devices to store parameters like the board serial number, factory
> > > MAC addresses and some other information.
> > > These parameters are usually written to the flash in the factory.
> > >
> > > Currently the mac command supports reading/writing parameters and dumping
> > > the current hw_info parameters.
> > > EEPROM config pattern and checksum aren't supported.
> > >
> > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > successfully, both reading the stock U-Boot parameters in mainline U-Boot
> > > and reading the parameters written by this command in the stock U-Boot.
> > >
> > > Usage example:
> > >  => mac read
> > >  => saveenv
> >
> > Hello Luka!
> >
> > You are just using above commands for accessing following variables,
> > right?
> >
> > pcb_slm
> > pcb_rev
> > eco_rev
> > pcb_sn
> > ethaddr
> > eth1addr
> > eth2addr
> > eth3addr
> > eth4addr
> > eth5addr
> > eth6addr
> > eth7addr
> > eth8addr
> > eth9addr
> >
> 
> Yes, I am accessing these variables.
> 
> > So why is additional command required? Espressobin boards can already
> > load and use correct eth*addr variables, which is implemented in board
> > file.
> 
> The current implementation in the board file preserves currently stored MAC
> addresses, which are in the U-Boot environment.

Yes, this is for v5/v7 variants. But it can be easily modified for ultra
to read mac addresses from this custom storage.

> My implementation is suitable for the ESPRESSOBin-Ultra, as the MACs
> are stored in the hw_info area of the SPI flash and the tool imports them
> into the normal U-Boot env.
> They can also be modified using the tool - we won't be able to get this
> functionality in the board file.

Modification by env variables is also possible from the board file. Look
at U_BOOT_ENV_CALLBACK() functionality, which allows to call custom code
when trying to change/delete particular env variable.

> > So what about implementing this import for all above variables for ultra
> > variant in board file, like it is already for all other variants? I have
> > already suggested it in the past. As I think this approach is just one
> > giant and complicated hack...
> > https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/
> >
> > I guess that it should be enough to import variables from SPI to env in
> > board_late_init() function.
> 
> Currently, the MACs are automatically imported from the
> common/board_r.c file at boot, I think this is more standard and cleaner.

Hm... then I somehow misunderstood how it works.

Because I do not see any change in common/board_r.c which could read or
import mac addresses via calling this new "mac read" command.

> Nevertheless, I believe this a sufficient solution for now,
> as this has been suggested as a solution in the previous patchset
> comments too.
> 
> >
> > > Signed-off-by: Luka Kovacic 
> > > Cc: Luka Perkov 
> > > Cc: Robert Marko 
> > > ---
> > >  arch/arm/mach-mvebu/Kconfig   |   1 +
> > >  board/Marvell/mvebu_armada-37xx/Kconfig   |  29 ++
> > >  board/Marvell/mvebu_armada-37xx/Makefile  |   3 +-
> > >  board/Marvell/mvebu_armada-37xx/mac/Makefile  |   5 +
> > >  board/Marvell/mvebu_armada-37xx/mac/hw_info.c | 391 ++
> > >  include/configs/mvebu_armada-37xx.h   |   7 +
> > >  lib/hashtable.c   |   2 +-
> > >  7 files changed, 436 insertions(+), 2 deletions(-)
> > >  create mode 100644 board/Marvell/mvebu_armada-37xx/Kconfig
> > >  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/Makefile
> > >  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> > >
> > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > > index 89737a37ad..dff9f05b28 100644
> > > --- a/arch/arm/mach-mvebu/Kconfig
> > > +++ b/arch/arm/mach-mvebu/Kconfig
> > > @@ -312,6 +312,7 @@ config SECURED_MODE_CSK_INDEX
> > >   default 0
> > >   depends on SECURED_MODE_IMAGE
> > >
> > > +source "board/Marvell/mvebu_armada-37xx/Kconfig"
> > >  source "board/solidrun/clearfog/Kconfig"
> > >  source "board/kobol/helios4/Kconfig"
> > >
> > > diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig 
> > > b/board/Marvell/mvebu_armada-37xx/Kconfig
> > > new file mode 100644
> > > index 00..b84dd20023
> > > --- /dev/null
> > > +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> > > @@ -0,0 +1,29 @@
> > > +menu "Marvell Armada 37xx configuration"
> > > +depends on TARGET_MVEBU_ARMADA_37XX
> > > +
> > > +config MVEBU_MAC_HW_INFO
> > > + bool "Marvell hw_info (mac) support"
> > > + depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEB

Re: [PATCH v4 3/3] arm64: Kconfig: Enable usage of optimized memset/memcpy/memmove

2021-08-13 Thread Rasmus Villemoes

On 12/08/2021 16.47, Stefan Roese wrote:
> This patch enables the use of the optimized memset(), memmove() &
> memcpy() versions recently added on ARM64.
> 
> Signed-off-by: Stefan Roese 
> 
>  
> +config USE_ARCH_MEMMOVE
> + bool "Use an assembly optimized implementation of memmove"
> + default y
> + depends on ARM64
> + help
> +   Enable the generation of an optimized version of memmove.
> +   Such an implementation may be faster under some conditions
> +   but may increase the binary size.

Hm. I don't think you can allow making this separately (de)selectable,
since if the optimized memcpy is selected, the memmove comes
unconditionally for free [and it would be a bit silly to guard the
ENTRY_ALIAS() by a CONFIG check IMO]. So with USE_ARCH_MEMCPY=y,
USE_ARCH_MEMMOVE=n, I think you'd get a "multiple definitions of
memmove" error. So on arm64, I think USE_ARCH_MEMMOVE should simply be
an unchangeable alias for USE_ARCH_MEMCPY (and similarly for the SPL/TPL
variants).

In Kconfig, I think one could spell this

  bool "Use an ..." if !ARM64
  default USE_ARCH_MEMCPY if ARM64
  depends on ARM64

where the last line can be removed if an when there's a separate memmove
for arm32 that one should be allowed to select.

Rasmus

Re: [PATCH v3 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

Hello Pali,

On Fri, Aug 13, 2021 at 11:31 AM Pali Rohár  wrote:
>
> On Friday 13 August 2021 10:59:33 Luka Kovacic wrote:
> > Hello Pali,
> >
> > On Fri, Aug 13, 2021 at 10:08 AM Pali Rohár  wrote:
> > >
> > > On Friday 13 August 2021 01:39:37 Luka Kovacic wrote:
> > > > Add the loadaddr U-Boot environment variable, as this is available in
> > > > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
> > >
> > > Hello Luka! Why is this change needed? Reason that it is in historic
> > > vendor U-Boot does not mean that it has to be also in new mainline
> > > version.
> > >
> > > I have already wrote some reasons in previous review thread:
> > > https://lore.kernel.org/u-boot/20210301150505.3iajeeufi7ahsnk3@pali/
> > >
> > > I think this change was needed when CONFIG_SYS_LOAD_ADDR was set to
> > > incorrect value, which is already fixed in mainline U-Boot.
> >
> > This value is very useful when building custom Linux boot scripts.
> > Yesterday, I booted the board without this patch and there was no loadaddr
> > variable.
> >
> > Do I understand this correctly? Are you saying that the value in the 
> > loadaddr
> > variable should be automatically inherited from CONFIG_SYS_LOAD_ADDR
> > even without this patch?
>
> If you do not specify load address then address from
> CONFIG_SYS_LOAD_ADDR is used. But as I mentioned in above email,
> CONFIG_SYS_LOAD_ADDR was defined incorrectly and which cause that it did
> not worked and caused crashes...

I tried not specifying it and then the loadaddr variable doesn't even exist.

>
> > >
> > > > Signed-off-by: Luka Kovacic 
> > > > Cc: Luka Perkov 
> > > > Cc: Robert Marko 
> > > > ---
> > > >  include/configs/mvebu_armada-37xx.h | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/include/configs/mvebu_armada-37xx.h 
> > > > b/include/configs/mvebu_armada-37xx.h
> > > > index 8e8bcfa018..6901680e32 100644
> > > > --- a/include/configs/mvebu_armada-37xx.h
> > > > +++ b/include/configs/mvebu_armada-37xx.h
> > > > @@ -110,6 +110,7 @@
> > > >
> > > >  /* fdt_addr and kernel_addr are needed for existing distribution boot 
> > > > scripts */
> > > >  #define CONFIG_EXTRA_ENV_SETTINGS\
> > > > + "loadaddr=0x600\0"  \
> > > >   "scriptaddr=0x6d0\0"\
> > > >   "pxefile_addr_r=0x6e0\0"\
> > > >   "fdt_addr=0x6f0\0"  \
> > > > --
> > > > 2.31.1
> > > >
> >
> > Kind regards,
> > Luka

Kind regards,
Luka

Re: [PATCH v3 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

On Friday 13 August 2021 10:59:33 Luka Kovacic wrote:
> Hello Pali,
> 
> On Fri, Aug 13, 2021 at 10:08 AM Pali Rohár  wrote:
> >
> > On Friday 13 August 2021 01:39:37 Luka Kovacic wrote:
> > > Add the loadaddr U-Boot environment variable, as this is available in
> > > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
> >
> > Hello Luka! Why is this change needed? Reason that it is in historic
> > vendor U-Boot does not mean that it has to be also in new mainline
> > version.
> >
> > I have already wrote some reasons in previous review thread:
> > https://lore.kernel.org/u-boot/20210301150505.3iajeeufi7ahsnk3@pali/
> >
> > I think this change was needed when CONFIG_SYS_LOAD_ADDR was set to
> > incorrect value, which is already fixed in mainline U-Boot.
> 
> This value is very useful when building custom Linux boot scripts.
> Yesterday, I booted the board without this patch and there was no loadaddr
> variable.
> 
> Do I understand this correctly? Are you saying that the value in the loadaddr
> variable should be automatically inherited from CONFIG_SYS_LOAD_ADDR
> even without this patch?

If you do not specify load address then address from
CONFIG_SYS_LOAD_ADDR is used. But as I mentioned in above email,
CONFIG_SYS_LOAD_ADDR was defined incorrectly and which cause that it did
not worked and caused crashes...

> >
> > > Signed-off-by: Luka Kovacic 
> > > Cc: Luka Perkov 
> > > Cc: Robert Marko 
> > > ---
> > >  include/configs/mvebu_armada-37xx.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/include/configs/mvebu_armada-37xx.h 
> > > b/include/configs/mvebu_armada-37xx.h
> > > index 8e8bcfa018..6901680e32 100644
> > > --- a/include/configs/mvebu_armada-37xx.h
> > > +++ b/include/configs/mvebu_armada-37xx.h
> > > @@ -110,6 +110,7 @@
> > >
> > >  /* fdt_addr and kernel_addr are needed for existing distribution boot 
> > > scripts */
> > >  #define CONFIG_EXTRA_ENV_SETTINGS\
> > > + "loadaddr=0x600\0"  \
> > >   "scriptaddr=0x6d0\0"\
> > >   "pxefile_addr_r=0x6e0\0"\
> > >   "fdt_addr=0x6f0\0"  \
> > > --
> > > 2.31.1
> > >
> 
> Kind regards,
> Luka

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> Technologies, Inc.
> 
> The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> Peripherals:
>  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
>  - RTC clock (PCF8563)
>  - USB 3.0 port
>  - USB 2.0 port
>  - 4x LED
>  - UART over Micro-USB
>  - M.2 slot (2280)
>  - Mini PCI-E slot
> 
> Additionally, automatic import of the Marvell hw_info parameters is
> enabled via the recently added mac command for A37XX platforms.
> The parameters stored in Marvell hw_info are usually the board serial
> number and MAC addresses.
> 
> Signed-off-by: Luka Kovacic 
> Cc: Luka Perkov 
> Cc: Robert Marko 
> ---
>  arch/arm/dts/Makefile |   1 +
>  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
>  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
>  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
>  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
>  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
>  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
>  7 files changed, 514 insertions(+), 203 deletions(-)
>  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
>  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
...
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c 
> b/board/Marvell/mvebu_armada-37xx/board.c
> index 2de9c2ac17..21c1eb7b22 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -55,6 +56,15 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define MVEBU_G2_SMI_PHY_CMD_REG (24)
>  #define MVEBU_G2_SMI_PHY_DATA_REG(25)
>  
> +/* Marvell 88E1512 */
> +#define MII_MARVELL_PHY_PAGE 22
> +
> +#define MV88E1512_GENERAL_CTRL   20
> +#define MV88E1512_MODE_SGMII 1
> +#define MV88E1512_RESET_OFFS 15
> +
> +#define ULTRA_MV88E1512_PHYADDR  0x1
> +
>  /*
>   * Memory Controller Registers
>   *
> @@ -282,12 +292,68 @@ static int mii_multi_chip_mode_write(struct mii_dev 
> *bus, int dev_smi_addr,
>   return 0;
>  }
>  
> -/* Bring-up board-specific network stuff */
> -int board_network_enable(struct mii_dev *bus)
> +void force_phy_88e1512_sgmii_to_copper(u16 devaddr)
>  {
> - if (!of_machine_is_compatible("globalscale,espressobin"))
> - return 0;
> + const char *name;
> + u16 reg;
> +
> + name = miiphy_get_current_dev();
> + if (name) {

It is possible that phy is not available? As you are calling code below
only in case name is not-NULL.

> + /* SGMII-to-Copper mode initialization */
> +
> + /* Select page 18 */
> + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0x12);
> + /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> + reg &= ~0x7;
> + reg |= MV88E1512_MODE_SGMII;
> + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> + /* PHY reset is necessary after changing MODE[2:0] */
> + miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, ®);
> + reg |= 1 << MV88E1512_RESET_OFFS;
> + miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
> + /* Reset page selection */
> + miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0);
> + udelay(100);
> + }
> +}
> +
> +int board_network_enable_espressobin_ultra(struct mii_dev *bus)
> +{
> + int i;
> + /* Setup 88E1512 SGMII-to-Copper mode */
> + force_phy_88e1512_sgmii_to_copper(ULTRA_MV88E1512_PHYADDR);
>  
> + /*
> +  * FIXME: remove this code once Topaz driver gets available
> +  * A3720 ESPRESSObin Ultra Board Only
> +  * Configure Topaz switch (88E6341)
> +  * Set port 1,2,3,4,5 to forwarding Mode (through Switch Port registers)
> +  */
> + for (i = 0; i <= 5; i++) {
> + mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(i),
> +   MVEBU_SW_PORT_CTRL_REG,
> +   i == 5 ? 0x7c : 0x7f);

Why port 5 has different settings?

> + }
> +
> + /* RGMII Delay on Port 0 (CPU port), force link to 1000Mbps */
> + mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(0),
> +   MVEBU_SW_LINK_CTRL_REG, 0xe002);
> +
> + /* Power up PHY 1, 2, 3, 4, 5 (through Global 2 registers) */
> + mii_multi_chip_mode_write(bus, 3, MVEBU_SW_G2_SMI_ADDR,
> +   MVEBU_G2_SMI_PHY_DATA_REG, 0x1140);
> + for (i = 1; i <= 5; i++) {
> +

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

Hello Marek and Pali,

On Fri, Aug 13, 2021 at 10:23 AM Pali Rohár  wrote:
>
> On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > The mac command is implemented to enable parsing Marvell hw_info formatted
> > environments. This format is often used on Marvell Armada A37XX based
> > devices to store parameters like the board serial number, factory
> > MAC addresses and some other information.
> > These parameters are usually written to the flash in the factory.
> >
> > Currently the mac command supports reading/writing parameters and dumping
> > the current hw_info parameters.
> > EEPROM config pattern and checksum aren't supported.
> >
> > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > successfully, both reading the stock U-Boot parameters in mainline U-Boot
> > and reading the parameters written by this command in the stock U-Boot.
> >
> > Usage example:
> >  => mac read
> >  => saveenv
>
> Hello Luka!
>
> You are just using above commands for accessing following variables,
> right?
>
> pcb_slm
> pcb_rev
> eco_rev
> pcb_sn
> ethaddr
> eth1addr
> eth2addr
> eth3addr
> eth4addr
> eth5addr
> eth6addr
> eth7addr
> eth8addr
> eth9addr
>

Yes, I am accessing these variables.

> So why is additional command required? Espressobin boards can already
> load and use correct eth*addr variables, which is implemented in board
> file.

The current implementation in the board file preserves currently stored MAC
addresses, which are in the U-Boot environment.

My implementation is suitable for the ESPRESSOBin-Ultra, as the MACs
are stored in the hw_info area of the SPI flash and the tool imports them
into the normal U-Boot env.
They can also be modified using the tool - we won't be able to get this
functionality in the board file.

>
> So what about implementing this import for all above variables for ultra
> variant in board file, like it is already for all other variants? I have
> already suggested it in the past. As I think this approach is just one
> giant and complicated hack...
> https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/
>
> I guess that it should be enough to import variables from SPI to env in
> board_late_init() function.

Currently, the MACs are automatically imported from the
common/board_r.c file at boot, I think this is more standard and cleaner.

Nevertheless, I believe this a sufficient solution for now,
as this has been suggested as a solution in the previous patchset
comments too.

>
> > Signed-off-by: Luka Kovacic 
> > Cc: Luka Perkov 
> > Cc: Robert Marko 
> > ---
> >  arch/arm/mach-mvebu/Kconfig   |   1 +
> >  board/Marvell/mvebu_armada-37xx/Kconfig   |  29 ++
> >  board/Marvell/mvebu_armada-37xx/Makefile  |   3 +-
> >  board/Marvell/mvebu_armada-37xx/mac/Makefile  |   5 +
> >  board/Marvell/mvebu_armada-37xx/mac/hw_info.c | 391 ++
> >  include/configs/mvebu_armada-37xx.h   |   7 +
> >  lib/hashtable.c   |   2 +-
> >  7 files changed, 436 insertions(+), 2 deletions(-)
> >  create mode 100644 board/Marvell/mvebu_armada-37xx/Kconfig
> >  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/Makefile
> >  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> >
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 89737a37ad..dff9f05b28 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -312,6 +312,7 @@ config SECURED_MODE_CSK_INDEX
> >   default 0
> >   depends on SECURED_MODE_IMAGE
> >
> > +source "board/Marvell/mvebu_armada-37xx/Kconfig"
> >  source "board/solidrun/clearfog/Kconfig"
> >  source "board/kobol/helios4/Kconfig"
> >
> > diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig 
> > b/board/Marvell/mvebu_armada-37xx/Kconfig
> > new file mode 100644
> > index 00..b84dd20023
> > --- /dev/null
> > +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> > @@ -0,0 +1,29 @@
> > +menu "Marvell Armada 37xx configuration"
> > +depends on TARGET_MVEBU_ARMADA_37XX
> > +
> > +config MVEBU_MAC_HW_INFO
> > + bool "Marvell hw_info (mac) support"
> > + depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> > + default n
> > + help
> > +   Enable loading of the Marvell hw_info parameters from the
> > +   SPI flash hw_info area. Parameters (usually the board serial
> > +   number and MAC addresses) are then imported into the
> > +   existing U-Boot environment.
> > +   Implementation of this command is compatible with the
> > +   original Marvell U-Boot command. Reading and writing is
> > +   supported.
> > +   EEPROM config pattern and checksum aren't supported.
> > +   After enabled, these parameters are managed from the common
> > +   U-Boot mac command.
> > +
> > +config MVEBU_MAC_HW_INFO_OFFSET
> > + hex "Marvell hw_info (mac) SPI flash offset"
> > + depends on MVEBU_MAC_HW_INFO
> > + default 0x3E
> > + he

Re: [PATCH] cmd: boot: Update reset usage message

2021-08-13 Thread Michal Simek

Hi Wolfgang,

On 8/13/21 8:54 AM, Wolfgang Denk wrote:
> Dear Michal,
> 
> In message <90e6c670-9e11-beb8-bcb5-9d22ba00f...@xilinx.com> you wrote:
>>
>>> In case of the hard (cold) reset - is it really only a reset of the
>>> CPU, or of the whole board hardware?
>>
>> If you look at sysreset headers you will find these levels
>>  11 SYSRESET_WARM,  /* Reset CPU, keep GPIOs active */
>>  12 SYSRESET_COLD,  /* Reset CPU and GPIOs */
>>  13 SYSRESET_POWER, /* Reset PMIC (remove and restore power) */
>>  14 SYSRESET_POWER_OFF, /* Turn off power */
>>
>> When you call reset sysreset uclass is calling sysreset_walk which is
>> request to drivers with type passed.
>> I see we have mixed drivers which deals with levels and especially in
>> gpio case it is question how you connect it.
>> I develop this for microblaze where gpio is connected reset logic which
>> is normally only for CPU and subsystem.
>> But in general you can connect whatever you want. It means it doesn't
>> need to be only cpu which is reset.
> 
> Thanks a lot for the explanation.
> 
>> Do you want me to update that line and remove CPU from it?
> 
> I don't know :-)
> 
> What the "reset" command _should_ do is a hard cold boot including
> the reset of all peripherals - for example, when booting from SPI
> NOR flash it is mandatory to reset this flash to make sure the ROM
> boot loader can actually read it.
> 
> I would appreciate if the help message documents what it actually
> does.  Also, what the difference between "cold" and "warm" reset is.
> My expectation (without knowing any hardware details) would be that
> a warm reset is just a restart of the already loaded code? Or is it
> just a reset of the CPU (without external reset)? This might then
> hang the system if it's attempting to boot from a flash which is in
> the wrong state.
> 
> So my problem with this is primarily that I don't understand what
> the command really does, and the help command is of no help either.
> 
> [And if I understand correctly, this is even board dependent?]

https://developer.arm.com/documentation/den0022/latest/
DEN0022D_b_Power_State_Coordination_Interface.pdf
Where 5.11.1 chapter is saying
"This function provides a method for performing a system cold reset. To
the caller, the behavior is
equivalent to a hardware power-cycle sequence."

And also 5.12.2.1 for SYSTEM_RESET2
5.12.2.1 System Warm reset
This reset is selected when reset_type is 0x0 (SYSTEM_WARM_RESET).
The function is intended to provide a fast reboot path that guarantees
not to reset system main memory.
This is defined as volatile memory accessible by cores when executing in
the calling operating system.
This definition does not extend to caches or to memory-mapped IO.
The memory that is to be preserved and not trashed by boot firmware
needs to be described by
firmware table technologies such as ACPI [6] and Device Tree [7] and
does not need to cover all
available system main memory.

But they also open vendor-specific resets.
It means defining resets is more or less implementation dependent.

Thanks,
Michal

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

Hello Pali,

On Fri, Aug 13, 2021 at 10:14 AM Pali Rohár  wrote:
>
> On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> > Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> > Technologies, Inc.
> >
> > The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> > Peripherals:
> >  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
> >  - RTC clock (PCF8563)
> >  - USB 3.0 port
> >  - USB 2.0 port
> >  - 4x LED
> >  - UART over Micro-USB
> >  - M.2 slot (2280)
> >  - Mini PCI-E slot
> >
> > Additionally, automatic import of the Marvell hw_info parameters is
> > enabled via the recently added mac command for A37XX platforms.
> > The parameters stored in Marvell hw_info are usually the board serial
> > number and MAC addresses.
> >
> > Signed-off-by: Luka Kovacic 
> > Cc: Luka Perkov 
> > Cc: Robert Marko 
> > ---
> >  arch/arm/dts/Makefile |   1 +
> >  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
> >  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
> >  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
> >  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
> >  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
> >  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
> >  7 files changed, 514 insertions(+), 203 deletions(-)
> >  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
> >  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> >  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig
>
> Hello Luka! Please look at my comments from previous review:
> https://lore.kernel.org/u-boot/20210227004852.5urcwnn6uxehuk72@pali/
>
> I think it is not a good idea to duplicate espressobin code, specially
> now when differences between v5, v7, non-emmc and emmc were
> de-duplicated and correctly detected at runtime. Just use one DTS and
> one config file and differences can be handled in board code functions
> "board_fix_fdt" and "board_late_init".

I believe that patching the DTS at runtime diminishes the value of
device trees in the first case.

As far as the v5, v7, non-emmc and emmc boards go I do understand
that, as they are physically similar and more or less different revisions
of the same board.

The ESPRESSOBin Ultra board is completely different from those boards
physically and so I doesn't make sense to me, why we would merge all
of them into one device tree.

I resorted to the Linux way and used a common dtsi for both the
ESPRESSOBin - which is the base and the ESPRESSOBin-Ultra.

>
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index c42715ead4..f21c9c94d3 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -213,6 +213,7 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
> >  dtb-$(CONFIG_ARCH_MVEBU) +=  \
> >   armada-3720-db.dtb  \
> >   armada-3720-espressobin.dtb \
> > + armada-3720-espressobin-ultra.dtb   \
> >   armada-3720-turris-mox.dtb  \
> >   armada-3720-uDPU.dtb\
> >   armada-375-db.dtb   \
> > diff --git a/arch/arm/dts/armada-3720-espressobin-ultra.dts 
> > b/arch/arm/dts/armada-3720-espressobin-ultra.dts
> > new file mode 100644
> > index 00..5ad0c723e3
> > --- /dev/null
> > +++ b/arch/arm/dts/armada-3720-espressobin-ultra.dts
> > @@ -0,0 +1,114 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Device Tree file for ESPRESSObin-Ultra board
> > + * Copyright (C) 2016 Marvell
> > + * Copyright (C) 2019 Globalscale technologies, Inc.
> > + * Copyright (C) 2021 Sartura Ltd.
> > + *
> > + * Author: Jason Hung 
> > + * Author: Luka Kovacic 
> > + * Author: Vladimir Vid 
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "armada-3720-espressobin.dtsi"
> > +
> > +/ {
> > + model = "Globalscale Marvell ESPRESSOBin Ultra Board";
> > + compatible = "globalscale,espressobin-ultra", "marvell,armada3720", 
> > "marvell,armada3710";
> > +
> > + gpio-leds {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&led1_pins>, <&led2_pins>, <&led3_pins>, 
> > <&led4_pins>;
> > + compatible = "gpio-leds";
> > +
> > + led1 {
> > + label = "led1";
> > + gpios = <&gpionb 11 GPIO_ACTIVE_LOW>;
> > + default-state = "on";
> > + };
> > + led2 {
> > + label = "led2";
> > + gpios = <&gpionb 12 GPIO_ACTIVE_LOW>;
> > + default-state = "on";
> > + };
> > + led3 {
> > + label = "led3";
> > + gpios = <&gpionb 13 GPIO_ACTIVE_LOW>;
> > + default-state = "on";
> > + };
> > + led4 {
> > + label = "led4";
>

Re: [PATCH v3 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

Hello Pali,

On Fri, Aug 13, 2021 at 10:08 AM Pali Rohár  wrote:
>
> On Friday 13 August 2021 01:39:37 Luka Kovacic wrote:
> > Add the loadaddr U-Boot environment variable, as this is available in
> > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
>
> Hello Luka! Why is this change needed? Reason that it is in historic
> vendor U-Boot does not mean that it has to be also in new mainline
> version.
>
> I have already wrote some reasons in previous review thread:
> https://lore.kernel.org/u-boot/20210301150505.3iajeeufi7ahsnk3@pali/
>
> I think this change was needed when CONFIG_SYS_LOAD_ADDR was set to
> incorrect value, which is already fixed in mainline U-Boot.

This value is very useful when building custom Linux boot scripts.
Yesterday, I booted the board without this patch and there was no loadaddr
variable.

Do I understand this correctly? Are you saying that the value in the loadaddr
variable should be automatically inherited from CONFIG_SYS_LOAD_ADDR
even without this patch?

>
> > Signed-off-by: Luka Kovacic 
> > Cc: Luka Perkov 
> > Cc: Robert Marko 
> > ---
> >  include/configs/mvebu_armada-37xx.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/include/configs/mvebu_armada-37xx.h 
> > b/include/configs/mvebu_armada-37xx.h
> > index 8e8bcfa018..6901680e32 100644
> > --- a/include/configs/mvebu_armada-37xx.h
> > +++ b/include/configs/mvebu_armada-37xx.h
> > @@ -110,6 +110,7 @@
> >
> >  /* fdt_addr and kernel_addr are needed for existing distribution boot 
> > scripts */
> >  #define CONFIG_EXTRA_ENV_SETTINGS\
> > + "loadaddr=0x600\0"  \
> >   "scriptaddr=0x6d0\0"\
> >   "pxefile_addr_r=0x6e0\0"\
> >   "fdt_addr=0x6f0\0"  \
> > --
> > 2.31.1
> >

Kind regards,
Luka

Re: [PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)

On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> The mac command is implemented to enable parsing Marvell hw_info formatted
> environments. This format is often used on Marvell Armada A37XX based
> devices to store parameters like the board serial number, factory
> MAC addresses and some other information.
> These parameters are usually written to the flash in the factory.
> 
> Currently the mac command supports reading/writing parameters and dumping
> the current hw_info parameters.
> EEPROM config pattern and checksum aren't supported.
> 
> This functionality has been tested on the GST ESPRESSOBin-Ultra board
> successfully, both reading the stock U-Boot parameters in mainline U-Boot
> and reading the parameters written by this command in the stock U-Boot.
> 
> Usage example:
>  => mac read
>  => saveenv

Hello Luka!

You are just using above commands for accessing following variables,
right?

pcb_slm
pcb_rev
eco_rev
pcb_sn
ethaddr
eth1addr
eth2addr
eth3addr
eth4addr
eth5addr
eth6addr
eth7addr
eth8addr
eth9addr

So why is additional command required? Espressobin boards can already
load and use correct eth*addr variables, which is implemented in board
file.

So what about implementing this import for all above variables for ultra
variant in board file, like it is already for all other variants? I have
already suggested it in the past. As I think this approach is just one
giant and complicated hack...
https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/

I guess that it should be enough to import variables from SPI to env in
board_late_init() function.

> Signed-off-by: Luka Kovacic 
> Cc: Luka Perkov 
> Cc: Robert Marko 
> ---
>  arch/arm/mach-mvebu/Kconfig   |   1 +
>  board/Marvell/mvebu_armada-37xx/Kconfig   |  29 ++
>  board/Marvell/mvebu_armada-37xx/Makefile  |   3 +-
>  board/Marvell/mvebu_armada-37xx/mac/Makefile  |   5 +
>  board/Marvell/mvebu_armada-37xx/mac/hw_info.c | 391 ++
>  include/configs/mvebu_armada-37xx.h   |   7 +
>  lib/hashtable.c   |   2 +-
>  7 files changed, 436 insertions(+), 2 deletions(-)
>  create mode 100644 board/Marvell/mvebu_armada-37xx/Kconfig
>  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/Makefile
>  create mode 100644 board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index 89737a37ad..dff9f05b28 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -312,6 +312,7 @@ config SECURED_MODE_CSK_INDEX
>   default 0
>   depends on SECURED_MODE_IMAGE
>  
> +source "board/Marvell/mvebu_armada-37xx/Kconfig"
>  source "board/solidrun/clearfog/Kconfig"
>  source "board/kobol/helios4/Kconfig"
>  
> diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig 
> b/board/Marvell/mvebu_armada-37xx/Kconfig
> new file mode 100644
> index 00..b84dd20023
> --- /dev/null
> +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> @@ -0,0 +1,29 @@
> +menu "Marvell Armada 37xx configuration"
> +depends on TARGET_MVEBU_ARMADA_37XX
> +
> +config MVEBU_MAC_HW_INFO
> + bool "Marvell hw_info (mac) support"
> + depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> + default n
> + help
> +   Enable loading of the Marvell hw_info parameters from the
> +   SPI flash hw_info area. Parameters (usually the board serial
> +   number and MAC addresses) are then imported into the
> +   existing U-Boot environment.
> +   Implementation of this command is compatible with the
> +   original Marvell U-Boot command. Reading and writing is
> +   supported.
> +   EEPROM config pattern and checksum aren't supported.
> +   After enabled, these parameters are managed from the common
> +   U-Boot mac command.
> +
> +config MVEBU_MAC_HW_INFO_OFFSET
> + hex "Marvell hw_info (mac) SPI flash offset"
> + depends on MVEBU_MAC_HW_INFO
> + default 0x3E
> + help
> +   This option defines the SPI flash offset of the Marvell
> +   hw_info area. This defaults to 0x3E on most Armada
> +   A3720 platforms.
> +
> +endmenu
> diff --git a/board/Marvell/mvebu_armada-37xx/Makefile 
> b/board/Marvell/mvebu_armada-37xx/Makefile
> index 27221557c7..6f6ac6b193 100644
> --- a/board/Marvell/mvebu_armada-37xx/Makefile
> +++ b/board/Marvell/mvebu_armada-37xx/Makefile
> @@ -2,4 +2,5 @@
>  #
>  # Copyright (C) 2016 Stefan Roese 
>  
> -obj-y:= board.o
> +obj-y += board.o
> +obj-y += mac/
> diff --git a/board/Marvell/mvebu_armada-37xx/mac/Makefile 
> b/board/Marvell/mvebu_armada-37xx/mac/Makefile
> new file mode 100644
> index 00..7c30fe6194
> --- /dev/null
> +++ b/board/Marvell/mvebu_armada-37xx/mac/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2021 Sartura Ltd.
> +
> +obj-$(CONFIG_ID_EEPROM) += hw_info.o
> diff --git a/board/Marvell/mvebu_armada-37xx/mac/hw_info.c 
> b/board/Marvell/

Re: [PATCH v3 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

On Friday 13 August 2021 01:39:38 Luka Kovacic wrote:
> Add initial support for the ESPRESSOBin-Ultra board from Globalscale
> Technologies, Inc.
> 
> The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
> Peripherals:
>  - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
>  - RTC clock (PCF8563)
>  - USB 3.0 port
>  - USB 2.0 port
>  - 4x LED
>  - UART over Micro-USB
>  - M.2 slot (2280)
>  - Mini PCI-E slot
> 
> Additionally, automatic import of the Marvell hw_info parameters is
> enabled via the recently added mac command for A37XX platforms.
> The parameters stored in Marvell hw_info are usually the board serial
> number and MAC addresses.
> 
> Signed-off-by: Luka Kovacic 
> Cc: Luka Perkov 
> Cc: Robert Marko 
> ---
>  arch/arm/dts/Makefile |   1 +
>  .../arm/dts/armada-3720-espressobin-ultra.dts | 114 ++
>  arch/arm/dts/armada-3720-espressobin.dts  | 199 +
>  arch/arm/dts/armada-3720-espressobin.dtsi | 210 ++
>  board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
>  board/Marvell/mvebu_armada-37xx/board.c   |  92 +++-
>  .../mvebu_espressobin-ultra-88f3720_defconfig |  93 
>  7 files changed, 514 insertions(+), 203 deletions(-)
>  create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
>  create mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>  create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig

Hello Luka! Please look at my comments from previous review:
https://lore.kernel.org/u-boot/20210227004852.5urcwnn6uxehuk72@pali/

I think it is not a good idea to duplicate espressobin code, specially
now when differences between v5, v7, non-emmc and emmc were
de-duplicated and correctly detected at runtime. Just use one DTS and
one config file and differences can be handled in board code functions
"board_fix_fdt" and "board_late_init".

> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index c42715ead4..f21c9c94d3 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -213,6 +213,7 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
>  dtb-$(CONFIG_ARCH_MVEBU) +=  \
>   armada-3720-db.dtb  \
>   armada-3720-espressobin.dtb \
> + armada-3720-espressobin-ultra.dtb   \
>   armada-3720-turris-mox.dtb  \
>   armada-3720-uDPU.dtb\
>   armada-375-db.dtb   \
> diff --git a/arch/arm/dts/armada-3720-espressobin-ultra.dts 
> b/arch/arm/dts/armada-3720-espressobin-ultra.dts
> new file mode 100644
> index 00..5ad0c723e3
> --- /dev/null
> +++ b/arch/arm/dts/armada-3720-espressobin-ultra.dts
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Device Tree file for ESPRESSObin-Ultra board
> + * Copyright (C) 2016 Marvell
> + * Copyright (C) 2019 Globalscale technologies, Inc.
> + * Copyright (C) 2021 Sartura Ltd.
> + *
> + * Author: Jason Hung 
> + * Author: Luka Kovacic 
> + * Author: Vladimir Vid 
> + */
> +
> +/dts-v1/;
> +
> +#include "armada-3720-espressobin.dtsi"
> +
> +/ {
> + model = "Globalscale Marvell ESPRESSOBin Ultra Board";
> + compatible = "globalscale,espressobin-ultra", "marvell,armada3720", 
> "marvell,armada3710";
> +
> + gpio-leds {
> + pinctrl-names = "default";
> + pinctrl-0 = <&led1_pins>, <&led2_pins>, <&led3_pins>, 
> <&led4_pins>;
> + compatible = "gpio-leds";
> +
> + led1 {
> + label = "led1";
> + gpios = <&gpionb 11 GPIO_ACTIVE_LOW>;
> + default-state = "on";
> + };
> + led2 {
> + label = "led2";
> + gpios = <&gpionb 12 GPIO_ACTIVE_LOW>;
> + default-state = "on";
> + };
> + led3 {
> + label = "led3";
> + gpios = <&gpionb 13 GPIO_ACTIVE_LOW>;
> + default-state = "on";
> + };
> + led4 {
> + label = "led4";
> + gpios = <&gpionb 14 GPIO_ACTIVE_LOW>;
> + default-state = "on";
> + };
> + };
> +};
> +
> +&pinctrl_nb {
> + led1_pins: led1-pins {
> + groups = "pwm0";
> + function = "gpio";
> + };
> + led2_pins: led2-pins {
> + groups = "pwm1";
> + function = "gpio";
> + };
> + led3_pins: led3-pins {
> + groups = "pwm2";
> + function = "gpio";
> + };
> + led4_pins: led4-pins {
> + groups = "pwm3";
> + function = "gpio";
> + };
> +};
> +
> +ð0 {
> + status = "okay";
> + phy_addr = <0x3>;
> +};
> +
> +&i2c0 {
> + status = "okay";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + rtc@51 {
> + compatible = "nxp,pcf85

Re: [PATCH v3 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

On Friday 13 August 2021 01:39:37 Luka Kovacic wrote:
> Add the loadaddr U-Boot environment variable, as this is available in
> the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.

Hello Luka! Why is this change needed? Reason that it is in historic
vendor U-Boot does not mean that it has to be also in new mainline
version.

I have already wrote some reasons in previous review thread:
https://lore.kernel.org/u-boot/20210301150505.3iajeeufi7ahsnk3@pali/

I think this change was needed when CONFIG_SYS_LOAD_ADDR was set to
incorrect value, which is already fixed in mainline U-Boot.

> Signed-off-by: Luka Kovacic 
> Cc: Luka Perkov 
> Cc: Robert Marko 
> ---
>  include/configs/mvebu_armada-37xx.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/mvebu_armada-37xx.h 
> b/include/configs/mvebu_armada-37xx.h
> index 8e8bcfa018..6901680e32 100644
> --- a/include/configs/mvebu_armada-37xx.h
> +++ b/include/configs/mvebu_armada-37xx.h
> @@ -110,6 +110,7 @@
>  
>  /* fdt_addr and kernel_addr are needed for existing distribution boot 
> scripts */
>  #define CONFIG_EXTRA_ENV_SETTINGS\
> + "loadaddr=0x600\0"  \
>   "scriptaddr=0x6d0\0"\
>   "pxefile_addr_r=0x6e0\0"\
>   "fdt_addr=0x6f0\0"  \
> -- 
> 2.31.1
>

Re: Hints on how to use efi_driver/efi_block_device.c

2021-08-13 Thread Christian Melki

On 8/13/21 2:36 AM, Heinrich Schuchardt wrote:

On 8/12/21 11:49 PM, Simon Glass wrote:

+Heinrich Schuchardt too

On Thu, 12 Aug 2021 at 08:35, Christian Melki
wrote:

Barebox does not have an emmc driver for the real hardware but
piggybacks off the UEFI protocol for this.

Hello Christian,

U-Boot can be used in two scenarios:

1) U-Boot is the firmware providing the UEFI API
2) U-Boot is running as an application consuming the UEFI API.

Barebox only supports scenario 2).

Ok.

I'm sure that the idea with this U-boot driver is something like that,
but would appreciate some hints on how to use it.

efi_driver/efi_block_device.c is used in scenario 1).

When a UEFI payload like iPXE provides an EFI_BLOCK_IO_PROTOCOL on a
handle and calls ConnectController() U-Boot will install the
EFI_SIMPLE_FILE_PROTOCOL for each partition on the block device.

You can find a detailed description of this use case in:

* https://u-boot.readthedocs.io/en/latest/develop/uefi/iscsi.html
* https://archive.fosdem.org/2020/schedule/event/firmware_duwu/

I read the fosdem presentation, but apparently did not understand it
correctly, as I thought it could be used for presenting the UEFI block
IO protocol.

Ie, standard usage. UEFI boots of a drive, posts some handles(?) to a
block device and U-boot picks it up, not knowing more about the
abstracted hardware.

Here you seem be referring to scenario 2).

Please, describe what you want to do with U-Boot.

So definitely an use-case for me.

Thanks in advance,
Christian

Best regards

Heinrich

[PATCH v4 5/5] efi_loader: add comment for efi_tcg2.h

This commit adds the comment of the TCG Specification
efi_tcg2.h file refers, and comment for the structure.

Signed-off-by: Masahisa Kojima 
---

(no change since v3)

Changes in v3:
- update comment format

Changes in v2:
- newly create commit from v2

 include/efi_tcg2.h | 57 +++---
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 497ba3ce94..b6b958da51 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -3,6 +3,13 @@
  * Defines data structures and APIs that allow an OS to interact with UEFI
  * firmware to query information about the device
  *
+ * This file refers the following TCG specification.
+ *  - TCG PC Client Platform Firmware Profile Specification
+ *
https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
+ *
+ *  - TCG EFI Protocol Specification
+ *
https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/
+ *
  * Copyright (c) 2020, Linaro Limited
  */
 
@@ -36,11 +43,23 @@ typedef u32 efi_tcg_event_log_bitmap;
 typedef u32 efi_tcg_event_log_format;
 typedef u32 efi_tcg_event_algorithm_bitmap;
 
+/**
+ * struct tdEFI_TCG2_VERSION - structure of EFI TCG2 version
+ * @major: major version
+ * @minor: minor version
+ */
 struct efi_tcg2_version {
u8 major;
u8 minor;
 };
 
+/**
+ * struct tdEFI_TCG2_EVENT_HEADER - structure of EFI TCG2 event header
+ * @header_size:   size of the event header
+ * @header_version:header version
+ * @pcr_index: index of the PCR that is extended
+ * @event_type:type of the event that is extended
+ */
 struct efi_tcg2_event_header {
u32 header_size;
u16 header_version;
@@ -48,12 +67,27 @@ struct efi_tcg2_event_header {
u32 event_type;
 } __packed;
 
+/**
+ * struct tdEFI_TCG2_EVENT - structure of EFI TCG2 event
+ * @size:  total size of the event including the size component, the header
+ * and the event data
+ * @header:event header
+ * @event: event to add
+ */
 struct efi_tcg2_event {
u32 size;
struct efi_tcg2_event_header header;
u8 event[];
 } __packed;
 
+/**
+ * struct tdUEFI_IMAGE_LOAD_EVENT - structure of PE/COFF image measurement
+ * @image_location_in_memory:  image address
+ * @image_length_in_memory:image size
+ * @image_link_time_address:   image link time address
+ * @length_of_device_path: devive path size
+ * @device_path:   device path
+ */
 struct uefi_image_load_event {
efi_physical_addr_t image_location_in_memory;
u64 image_length_in_memory;
@@ -62,6 +96,23 @@ struct uefi_image_load_event {
struct efi_device_path device_path[];
 };
 
+/**
+ * struct tdEFI_TCG2_BOOT_SERVICE_CAPABILITY - protocol capability information
+ * @size:  allocated size of the structure
+ * @structure_version: version of this structure
+ * @protocol_version:  version of the EFI TCG2 protocol.
+ * @hash_algorithm_bitmap: supported hash algorithms
+ * @supported_event_logs:  bitmap of supported event log formats
+ * @tpm_present_flag:  false = TPM not present
+ * @max_command_size:  max size (in bytes) of a command
+ * that can be sent to the TPM
+ * @max_response_size: max size (in bytes) of a response that
+ * can be provided by the TPM
+ * @manufacturer_id:   4-byte Vendor ID
+ * @number_of_pcr_banks:   maximum number of PCR banks
+ * @active_pcr_banks:  bitmap of currently active
+ * PCR banks (hashing algorithms).
+ */
 struct efi_tcg2_boot_service_capability {
u8 size;
struct efi_tcg2_version structure_version;
@@ -86,7 +137,7 @@ struct efi_tcg2_boot_service_capability {
 #define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2 2
 
 /**
- *  struct TCG_EfiSpecIdEventAlgorithmSize
+ *  struct TCG_EfiSpecIdEventAlgorithmSize - hashing algorithm information
  *
  *  @algorithm_id: algorithm defined in enum tpm2_algorithms
  *  @digest_size:  size of the algorithm
@@ -97,7 +148,7 @@ struct tcg_efi_spec_id_event_algorithm_size {
 } __packed;
 
 /**
- * struct TCG_EfiSpecIDEventStruct
+ * struct TCG_EfiSpecIDEventStruct - content of the event log header
  *
  * @signature: signature, set to Spec ID Event03
  * @platform_class:class defined in TCG ACPI Specification
@@ -130,7 +181,7 @@ struct tcg_efi_spec_id_event {
 } __packed;
 
 /**
- * struct tdEFI_TCG2_FINAL_EVENTS_TABLE
+ * struct tdEFI_TCG2_FINAL_EVENTS_TABLE - log entries after Get Event Log
  * @version:   version number for this structure
  * @number_of_events:  number of events recorded after invocation of
  * GetEventLog()
-- 
2.17.1

[PATCH v4 4/5] efi_loader: refactor efi_append_scrtm_version()

Refactor efi_append_scrtm_version() to use common
function for adding eventlog and extending PCR.

Signed-off-by: Masahisa Kojima 
---

(no changes since v1)


 lib/efi_loader/efi_tcg2.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 8557fce1da..2d03809e85 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1321,23 +1321,11 @@ out:
  */
 static efi_status_t efi_append_scrtm_version(struct udevice *dev)
 {
-   struct tpml_digest_values digest_list;
u8 ver[] = U_BOOT_VERSION_STRING;
-   const int pcr_index = 0;
efi_status_t ret;
 
-   ret = tcg2_create_digest(ver, sizeof(ver), &digest_list);
-   if (ret != EFI_SUCCESS)
-   goto out;
+   ret = tcg2_measure_event(dev, 0, EV_S_CRTM_VERSION, sizeof(ver), ver);
 
-   ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
-   if (ret != EFI_SUCCESS)
-   goto out;
-
-   ret = tcg2_agile_log_append(pcr_index, EV_S_CRTM_VERSION, &digest_list,
-   sizeof(ver), ver);
-
-out:
return ret;
 }
 
-- 
2.17.1

[PATCH v4 3/5] efi_loader: add ExitBootServices() measurement

TCG PC Client PFP spec requires to measure
"Exit Boot Services Invocation" if ExitBootServices() is invoked.
Depending upon the return code from the ExitBootServices() call,
"Exit Boot Services Returned with Success" or "Exit Boot Services
Returned with Failure" is also measured.

Signed-off-by: Masahisa Kojima 
---
Changes in v4:
- remove unnecessary EFIAPI specifier

Changes in v2:
- use strlen instead of sizeof, event log for EV_EFI_ACTION string

  shall not include NUL terminator
 include/efi_loader.h  |  1 +
 lib/efi_loader/efi_boottime.c |  5 +++
 lib/efi_loader/efi_tcg2.c | 70 +++
 3 files changed, 76 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 6f61e9faac..32cb8d0f1e 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -499,6 +499,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t 
source_size);
 efi_status_t efi_init_variables(void);
 /* Notify ExitBootServices() is called */
 void efi_variables_boot_exit_notify(void);
+efi_status_t efi_tcg2_notify_exit_boot_services_failed(void);
 /* Measure efi application invocation */
 efi_status_t efi_tcg2_measure_efi_app_invocation(void);
 /* Measure efi application exit */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 13ab139222..b818cbb540 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2182,6 +2182,11 @@ static efi_status_t EFIAPI 
efi_exit_boot_services(efi_handle_t image_handle,
efi_set_watchdog(0);
WATCHDOG_RESET();
 out:
+   if (ret != EFI_SUCCESS) {
+   if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL))
+   efi_tcg2_notify_exit_boot_services_failed();
+   }
+
return EFI_EXIT(ret);
 }
 
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index ed71337780..8557fce1da 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1506,6 +1506,67 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
return ret;
 }
 
+/**
+ * efi_tcg2_notify_exit_boot_services() - ExitBootService callback
+ *
+ * @event: callback event
+ * @context:   callback context
+ */
+static void
+efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
+{
+   efi_status_t ret;
+   struct udevice *dev;
+
+   EFI_ENTRY("%p, %p", event, context);
+
+   ret = platform_get_tpm2_device(&dev);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
+(u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+strlen(EFI_EXIT_BOOT_SERVICES_SUCCEEDED),
+(u8 *)EFI_EXIT_BOOT_SERVICES_SUCCEEDED);
+
+out:
+   EFI_EXIT(ret);
+}
+
+/**
+ * efi_tcg2_notify_exit_boot_services_failed()
+ *  - notify ExitBootServices() is failed
+ *
+ * Return: status code
+ */
+efi_status_t efi_tcg2_notify_exit_boot_services_failed(void)
+{
+   struct udevice *dev;
+   efi_status_t ret;
+
+   ret = platform_get_tpm2_device(&dev);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
+(u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,
+strlen(EFI_EXIT_BOOT_SERVICES_FAILED),
+(u8 *)EFI_EXIT_BOOT_SERVICES_FAILED);
+
+out:
+   return ret;
+}
+
 /**
  * tcg2_measure_secure_boot_variable() - measure secure boot variables
  *
@@ -1584,6 +1645,7 @@ efi_status_t efi_tcg2_register(void)
 {
efi_status_t ret = EFI_SUCCESS;
struct udevice *dev;
+   struct efi_event *event;
 
ret = platform_get_tpm2_device(&dev);
if (ret != EFI_SUCCESS) {
@@ -1608,6 +1670,14 @@ efi_status_t efi_tcg2_register(void)
goto fail;
}
 
+   ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+  efi_tcg2_notify_exit_boot_services, NULL,
+  NULL, &event);
+   if (ret != EFI_SUCCESS) {
+   tcg2_uninit();
+   goto fail;
+   }
+
ret = tcg2_measure_secure_boot_variable(dev);
if (ret != EFI_SUCCESS) {
tcg2_uninit();
-- 
2.17.1

[PATCH v4 2/5] efi_loader: add boot variable measurement

TCG PC Client PFP spec requires to measure "Boot"
and "BootOrder" variables, EV_SEPARATOR event prior
to the Ready to Boot invocation.
Since u-boot does not implement Ready to Boot event,
these measurements are performed when efi_start_image() is called.

TCG spec also requires to measure "Calling EFI Application from
Boot Option" for each boot attempt, and "Returning from EFI
Application from Boot Option" if a boot device returns control
back to the Boot Manager.

Signed-off-by: Masahisa Kojima 
---
Changes in v4:
- remove unnecessary EFIAPI specifier

Changes in v3:
- modify log output

Changes in v2:
- use efi_create_indexed_name() for "Boot" variable

 include/efi_loader.h  |   4 ++
 include/tpm-v2.h  |  18 -
 lib/efi_loader/efi_boottime.c |  20 ++
 lib/efi_loader/efi_tcg2.c | 121 ++
 4 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index a120d94431..6f61e9faac 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -499,6 +499,10 @@ efi_status_t efi_run_image(void *source_buffer, 
efi_uintn_t source_size);
 efi_status_t efi_init_variables(void);
 /* Notify ExitBootServices() is called */
 void efi_variables_boot_exit_notify(void);
+/* Measure efi application invocation */
+efi_status_t efi_tcg2_measure_efi_app_invocation(void);
+/* Measure efi application exit */
+efi_status_t efi_tcg2_measure_efi_app_exit(void);
 /* Called by bootefi to initialize root node */
 efi_status_t efi_root_node_register(void);
 /* Called by bootefi to initialize runtime */
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index 247b386967..325c73006e 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -73,7 +73,7 @@ struct udevice;
 /*
  * event types, cf.
  * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
- * rev 1.04, June 3, 2019
+ * Level 00 Version 1.05 Revision 23, May 7, 2021
  */
 #define EV_EFI_EVENT_BASE  ((u32)0x8000)
 #define EV_EFI_VARIABLE_DRIVER_CONFIG  ((u32)0x8001)
@@ -85,8 +85,24 @@ struct udevice;
 #define EV_EFI_ACTION  ((u32)0x8007)
 #define EV_EFI_PLATFORM_FIRMWARE_BLOB  ((u32)0x8008)
 #define EV_EFI_HANDOFF_TABLES  ((u32)0x8009)
+#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x800A)
+#define EV_EFI_HANDOFF_TABLES2 ((u32)0x800B)
+#define EV_EFI_VARIABLE_BOOT2  ((u32)0x800C)
 #define EV_EFI_HCRTM_EVENT ((u32)0x8010)
 #define EV_EFI_VARIABLE_AUTHORITY  ((u32)0x80E0)
+#define EV_EFI_SPDM_FIRMWARE_BLOB  ((u32)0x80E1)
+#define EV_EFI_SPDM_FIRMWARE_CONFIG((u32)0x80E2)
+
+#define EFI_CALLING_EFI_APPLICATION \
+   "Calling EFI Application from Boot Option"
+#define EFI_RETURNING_FROM_EFI_APPLICATION  \
+   "Returning from EFI Application from Boot Option"
+#define EFI_EXIT_BOOT_SERVICES_INVOCATION   \
+   "Exit Boot Services Invocation"
+#define EFI_EXIT_BOOT_SERVICES_FAILED   \
+   "Exit Boot Services Returned with Failure"
+#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED\
+   "Exit Boot Services Returned with Success"
 
 /* TPMS_TAGGED_PROPERTY Structure */
 struct tpms_tagged_property {
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 0b98e91813..13ab139222 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2994,6 +2994,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t 
image_handle,
image_obj->exit_status = &exit_status;
image_obj->exit_jmp = &exit_jmp;
 
+   if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
+   if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
+   ret = efi_tcg2_measure_efi_app_invocation();
+   if (ret != EFI_SUCCESS) {
+   log_warning("tcg2 measurement fails(0x%lx)\n",
+   ret);
+   }
+   }
+   }
+
/* call the image! */
if (setjmp(&exit_jmp)) {
/*
@@ -3252,6 +3262,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t 
image_handle,
exit_status != EFI_SUCCESS)
efi_delete_image(image_obj, loaded_image_protocol);
 
+   if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
+   if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
+   ret = efi_tcg2_measure_efi_app_exit();
+   if (ret != EFI_SUCCESS) {
+   log_warning("tcg2 measurement fails(0x%lx)\n",
+   ret);
+   }
+   }
+   }
+
/* Make sure entry/exit counts for EFI world cross-overs match */
EFI_EXIT(exit_status);
 
diff --git a/lib/efi_loader/efi_tcg

[PATCH v4 1/5] efi_loader: add secure boot variable measurement

TCG PC Client PFP spec requires to measure the secure
boot policy before validating the UEFI image.
This commit adds the secure boot variable measurement
of "SecureBoot", "PK", "KEK", "db", "dbx", "dbt", and "dbr".

Note that this implementation assumes that secure boot
variables are pre-configured and not be set/updated in runtime.

Signed-off-by: Masahisa Kojima 
---
Changes in v4:
- remove unnecessary EFIAPI specifier
- modify wrong guid for dbt and dbr

Changes in v3:
- add "dbt" and "dbr" measurement
- accept empty variable measurement for "SecureBoot", "PK",
  "KEK", "db" and "dbx" as TCG2 spec requires
- fix comment format

Changes in v2:
- missing null check for getting variable data
- some minor fix for readability

 include/efi_tcg2.h|  20 +
 lib/efi_loader/efi_tcg2.c | 165 ++
 2 files changed, 185 insertions(+)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index bcfb98168a..497ba3ce94 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
struct tcg_pcr_event2 event[];
 };
 
+/**
+ * struct tdUEFI_VARIABLE_DATA - event log structure of UEFI variable
+ * @variable_name: The vendorGUID parameter in the
+ * GetVariable() API.
+ * @unicode_name_length:   The length in CHAR16 of the Unicode name of
+ * the variable.
+ * @variable_data_length:  The size of the variable data.
+ * @unicode_name:  The CHAR16 unicode name of the variable
+ * without NULL-terminator.
+ * @variable_data: The data parameter of the efi variable
+ * in the GetVariable() API.
+ */
+struct efi_tcg2_uefi_variable_data {
+   efi_guid_t variable_name;
+   u64 unicode_name_length;
+   u64 variable_data_length;
+   u16 unicode_name[1];
+   u8 variable_data[1];
+};
+
 struct efi_tcg2_protocol {
efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
   struct 
efi_tcg2_boot_service_capability *capability);
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 1319a8b378..db3d6d7586 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
},
 };
 
+struct variable_info {
+   u16 *name;
+   const efi_guid_t*guid;
+};
+
+static struct variable_info secure_variables[] = {
+   {L"SecureBoot", &efi_global_variable_guid},
+   {L"PK", &efi_global_variable_guid},
+   {L"KEK", &efi_global_variable_guid},
+   {L"db", &efi_guid_image_security_database},
+   {L"dbx", &efi_guid_image_security_database},
+};
+
 #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
 
 /**
@@ -1264,6 +1277,39 @@ free_pool:
return ret;
 }
 
+/**
+ * tcg2_measure_event() - common function to add event log and extend PCR
+ *
+ * @dev:   TPM device
+ * @pcr_index: PCR index
+ * @event_type:type of event added
+ * @size:  event size
+ * @event: event data
+ *
+ * Return: status code
+ */
+static efi_status_t
+tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
+  u32 size, u8 event[])
+{
+   struct tpml_digest_values digest_list;
+   efi_status_t ret;
+
+   ret = tcg2_create_digest(event, size, &digest_list);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
+   size, event);
+
+out:
+   return ret;
+}
+
 /**
  * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
  *   eventlog and extend the PCRs
@@ -1294,6 +1340,118 @@ out:
return ret;
 }
 
+/**
+ * tcg2_measure_variable() - add variable event log and extend PCR
+ *
+ * @dev:   TPM device
+ * @pcr_index: PCR index
+ * @event_type:type of event added
+ * @var_name:  variable name
+ * @guid:  guid
+ * @data_size: variable data size
+ * @data:  variable data
+ *
+ * Return: status code
+ */
+static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
+ u32 event_type, u16 *var_name,
+ const efi_guid_t *guid,
+ efi_uintn_t data_size, u8 *data)
+{
+   u32 event_size;
+   efi_status_t ret;
+   struct efi_tcg2_uefi_variable_data *event;
+
+   event_size = sizeof(event->variable_name) +
+sizeof(event->unicode_name_length) +
+sizeof(event->

[PATCH v4 0/5] add measurement support