[GIT PULL] please pull NXP i.MX

[Peng Fan]

Hi Stefano,

This is collect of patches that pending in patchwork and some new patches 
posted recently.
The PR is rebased on Tom's master branch since imx/master was not up to date, 
should I do the PR based on imx/master?
CI: https://travis-ci.org/github/MrVan/u-boot/builds/707889918


i.MX DDR driver fix/update for i.MX8M
i.MX pinctrl driver fix.
Use arm_smccc_smc to remove imx sip function
i.MX8M clk update
support booting aarch32 kernel on aarch64 hardware
fused part support for i.MX8MP


Thanks,
Peng.

The following changes since commit 497c7598c4e713eb9ad88fd7963e57b21b8b35e1:

  Merge branch 'master' of https://gitlab.denx.de/u-boot/custodians/u-boot-spi 
(2020-07-11 17:40:00 -0400)

are available in the Git repository at:

  https://github.com/MrVan/u-boot.git tags/nxp-imx-7-14-2020

for you to fetch changes up to 86e9d7e8143eab4cfe4b74628153af9006bb1ad2:

  imx8mm_evk: enlarge CONFIG_SYS_BOOTM_LEN (2020-07-14 15:23:48 +0800)


Jacky Bai (2):
  driver: ddr: imx: skip ddr_ss_gpr config on imx8mn
  driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m

Jian Li (3):
  imx8mp: enable rd_port_urgent
  imx8mp: DDR performance tunning
  imx8mp: Disables use of MR4 TUF flag (MR4[7]) bit

Oliver Chen (1):
  drivers: ddr: imx Workaround for i.MX8M DDRPHY rank to rank issue

Peng Fan (22):
  pinctrl: imx7: move soc info to data section
  pinctrl: imx8m: move soc info to data section
  pinctrl: imx5: move soc info to data section
  imx8: misc: use arm_smccc_smc
  imx8m: soc: use arm_smccc_smc
  imx: bootaux: use arm_smccc_smc
  imx8: fuse: use arm_smccc_smc
  imx: power-domain: use arm_smccc_smc
  imx: remove imx sip file
  clk: imx8mm: fix clk set parent
  clk: imx8mm: Add qspi clock
  imx8m: configure arm clk sources from PLL
  imx8m: configure NoC clk
  imx8m: add sdhc/nand/ecspi clk api
  imx8m: add eqos clk
  imx8m: workaround ROM serror
  imx8m: power down fused cores
  imx8m: disable nodes before kernel/mfgtool boot for fused part
  clk: imx8m: drop clk settings
  imx8m: Refactor the OPTEE memory removal
  imx8m: implement armv8_el2_to_aarch32
  imx8mm_evk: enlarge CONFIG_SYS_BOOTM_LEN

Sherry Sun (1):
  drivers: ddr: imx8mp: Add inline ECC feature support

Ye Li (5):
  clk: clk-imx8mn: Update clock tree and support set parent
  clk: imx8mm/8mn: Add USB clocks
  clk: imx8mp: Update imx8mp ccf clock driver
  imx8mp: Add fused parts support
  imx8mn/imx8mp: override env_get_offset and env_get_location

 arch/arm/include/asm/arch-imx/cpu.h|   5 ++
 arch/arm/include/asm/arch-imx8m/ddr.h  |  10 +++
 arch/arm/include/asm/arch-imx8m/imx-regs.h | 158 
+
 arch/arm/include/asm/mach-imx/sys_proto.h  |   9 +-
 arch/arm/mach-imx/Makefile |   2 +-
 arch/arm/mach-imx/cpu.c|  12 ++-
 arch/arm/mach-imx/imx8/misc.c  |   7 +-
 arch/arm/mach-imx/imx8m/clock_imx8mm.c | 299 
-
 arch/arm/mach-imx/imx8m/lowlevel_init.S|  12 +++
 arch/arm/mach-imx/imx8m/soc.c  | 612 
++---
 arch/arm/mach-imx/imx_bootaux.c|  11 ++-
 arch/arm/mach-imx/sip.c|  48 --
 board/beacon/imx8mm/imx8mm_beacon.c|  11 ---
 board/freescale/imx8mm_evk/imx8mm_evk.c|  11 ---
 board/freescale/imx8mn_evk/imx8mn_evk.c|   7 --
 board/freescale/imx8mp_evk/imx8mp_evk.c|  40 -
 board/freescale/imx8mp_evk/lpddr4_timing.c |   5 +-
 board/freescale/imx8mq_evk/imx8mq_evk.c|  11 ---
 board/google/imx8mq_phanbell/imx8mq_phanbell.c |  11 ---
 board/technexion/pico-imx8mq/pico-imx8mq.c |  26 +++---
 board/toradex/verdin-imx8mm/verdin-imx8mm.c|  11 ---
 configs/imx8mp_evk_defconfig   |   1 -
 drivers/clk/imx/clk-imx8mm.c   |  63 ++---
 drivers/clk/imx/clk-imx8mn.c   | 108 +++---
 drivers/clk/imx/clk-imx8mp.c   |  52 +++
 drivers/ddr/imx/imx8m/Kconfig  |   7 ++
 drivers/ddr/imx/imx8m/ddr_init.c   |  79 -
 drivers/ddr/imx/imx8m/ddrphy_train.c   |   7 ++
 drivers/ddr/imx/imx8m/ddrphy_utils.c   | 164 
++
 drivers/misc/imx8/fuse.c   |  19 ++--
 drivers/pinctrl/nxp/pinctrl-imx5.c |   2 +-
 drivers/pinctrl/nxp/pinctrl-imx7.c |   2 +-
 drivers/pinctrl/nxp/pinctrl-imx8m.c|   2 +-
 drivers/power/domain/imx8m

RE: [PATCH] mmc: Parse new binding for eMMC fixed driver type

[Peng Fan]

> Subject: [PATCH] mmc: Parse new binding for eMMC fixed driver type
> 
> Parse the new binding and store it in the mmc config struct after doing some
> sanity checks. The code is designed to support fixed mmc driver type if we
> ever need that.
> 
> Signed-off-by: Shunsuke Tokumoto 
> Signed-off-by: Yasushi Iida 
> ---
>  drivers/mmc/mmc-uclass.c | 30 ++
>  drivers/mmc/mmc.c|  8 
>  include/mmc.h|  3 +++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index
> c5b7872900..e67aae451f 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -162,6 +162,36 @@ int dm_mmc_deferred_probe(struct udevice *dev)
>   if (ops->deferred_probe)
>   return ops->deferred_probe(dev);
> 
> + /* Check eMMC driver type selection */
> + val = dev_read_u32_default(dev, "fixed-emmc-driver-type", 0);

Has the bindings been accepted by Linux Kernel?

Regards,
Peng.

[GIT PULL] please pull mmc-7-24-2020

[Peng Fan]

Hi Tom,

Please pull mmc-7-24-2020


Correct mmc_spi check condition
Generate R1/R2/R1b response
Read SSR for SD SPI


CI: https://travis-ci.org/github/MrVan/u-boot/builds/707901966

Thanks,
Peng.

The following changes since commit 497c7598c4e713eb9ad88fd7963e57b21b8b35e1:

  Merge branch 'master' of https://gitlab.denx.de/u-boot/custodians/u-boot-spi 
(2020-07-11 17:40:00 -0400)

are available in the Git repository at:

  https://gitlab.denx.de/u-boot/custodians/u-boot-mmc.git tags/mmc-7-24-2020

for you to fetch changes up to ed4a11cb7d19a26dd0625bc468ef930c035a71b2:

  mmc_spi: generate R1b response for erase and stop transmission command 
(2020-07-14 16:19:47 +0800)


Pragnesh Patel (6):
  mmc: mmc_spi: correct the while condition
  mmc: mmc_spi: generate R1 response for different mmc SPI commands
  mmc: read ssr for SD spi
  mmc: mmc_spi: Read R2 response for send status command - CMD13
  mmc: mmc_spi: Generate R1 response for erase block start and end address
  mmc_spi: generate R1b response for erase and stop transmission command

 drivers/mmc/mmc.c |  5 +
 drivers/mmc/mmc_spi.c | 52 
 2 files changed, 49 insertions(+), 8 deletions(-)

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Peter Robinson]

On Tue, Jul 14, 2020 at 12:02 AM Simon South  wrote:
>
> Has anyone managed to get the built-in keyboard of the Pinebook Pro
> working with U-Boot?

It should be fixed in the main devel repo, commit 3a5771249

> Kever, Jagan: Are you aware of any special setup required to have the
> RK3399's OHCI controller begin processing its periodic list?
>
> Even using the latest code, having USB started makes the U-boot console
> feel sluggish while pressing keys on the keyboard produces no result.
>
> The issue seems to be the OHCI (USB 1.1) driver continually times out
> waiting for the controller to start an interrupt transfer (to poll the
> keyboard for a keypress). Dumping the OHCI controller's registers as
> well as the endpoint and transfer descriptors shows everything set up
> correctly, however, as best I can tell from the OHCI spec: The
> descriptors have the right values, the ED is added to the first
> interrupt list, and the controller even "sees" the ED (the
> HcPeriodCurrentED register holds its address once per frame). Yet when
> the timeout expires the TD remains unprocessed, with its condition code
> still set to "not accessed" and the controller's error counters still at
> zero.
>
> Oddly, control messages are passed to the keyboard just fine. It's as
> though the controller is simply ignoring the periodic list, even though
> the bit to enable its processing is set in the HcControl register.
>
> Plugging in an external keyboard (after updating the build configuration
> to include the right phy driver) produces the same result, so it's not
> just the built-in one. And obviously the OHCI driver works on other
> platforms, so it seems to me this could be something specific to
> Rockchip's implementation not yet reflected in the code.
>
> Has anyone found a solution to this?
>
> --
> Simon South
> si...@simonsouth.net

[PATCH v5 0/5] roc-rk3399-pc: Custom SPL

[Jagan Teki]

This series supports custom initialization code required for 
roc-rk3399-pc board on SPL stage.

Now this series is well mature code handling to add custom 
spl_board_init code parts.

roc-rk3399-pc would require custom leds initialization based 
on user intervention of the power key. This code handles the 
user intervention via SPI environment variable. If someone 
or production systems wants this feature then 'pwr_key' has 
to be set otherwise it is normal like other rk3399 boards 
in Mainline.

Changes for v5:
- drop banner changes
- add code changes in roc-pc-rk3399.c
Changes for v4:
- enable SPL_BOARD_INIT in all platforms
Changes for v3:
- support leds on SPL
- support env 'pwr_key'

Jagan Teki (5):
  roc-rk3399-pc: Move leds setup in SPL
  rockchip: Don't clear the reset status reg
  rockchip: Separate the reset cause from display cpuinfo
  rockchip: spl: Move board_early_init_f after cpu timer
  roc-rk3399-pc: Set LED only during POR and pwr_key=y

 arch/arm/include/asm/arch-rockchip/cru.h|  3 +-
 arch/arm/mach-rockchip/Makefile |  5 +-
 arch/arm/mach-rockchip/cpu-info.c   | 26 
 arch/arm/mach-rockchip/spl.c|  5 +-
 arch/arm/mach-rockchip/tpl.c|  7 ---
 board/firefly/roc-pc-rk3399/roc-pc-rk3399.c | 67 -
 configs/roc-pc-mezzanine-rk3399_defconfig   |  5 +-
 configs/roc-pc-rk3399_defconfig |  5 +-
 8 files changed, 80 insertions(+), 43 deletions(-)

-- 
2.25.1

[PATCH v5 1/5] roc-rk3399-pc: Move leds setup in SPL

[Jagan Teki]

roc-rk3399-pc has some specific requirements to support LEDS,
environment. board detection and etc prior to U-Boot proper.

So as of now SPL would be a better stage for these custom board
requirements to support unlike TPL. Adding few of these custom
requirements like LEDS in TPL would require extra code pulling
and also the size of TPL can grow.

So, this patch moves the leds code from TPL into SPL after relocation.

Signed-off-by: Jagan Teki 
---
Changes for v5
- drop tpl.c file
- update the code in board file

 arch/arm/mach-rockchip/tpl.c|  7 
 board/firefly/roc-pc-rk3399/roc-pc-rk3399.c | 36 +++--
 configs/roc-pc-mezzanine-rk3399_defconfig   |  2 +-
 configs/roc-pc-rk3399_defconfig |  2 +-
 4 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c
index 88f80b05a9..cc908e1b0e 100644
--- a/arch/arm/mach-rockchip/tpl.c
+++ b/arch/arm/mach-rockchip/tpl.c
@@ -43,18 +43,11 @@ __weak void rockchip_stimer_init(void)
   TIMER_CONTROL_REG);
 }
 
-__weak int board_early_init_f(void)
-{
-   return 0;
-}
-
 void board_init_f(ulong dummy)
 {
struct udevice *dev;
int ret;
 
-   board_early_init_f();
-
 #if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL_SUPPORT)
/*
 * Debug UART can be used from here if required:
diff --git a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c 
b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
index 7c3a803654..4db3dd739c 100644
--- a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
+++ b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
@@ -6,14 +6,24 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
-#include 
+#include 
+
 #include 
 
-#ifndef CONFIG_SPL_BUILD
-int board_early_init_f(void)
+#define GPIO0_BASE 0xff72
+
+static int led_setup(void)
+{
+   struct rockchip_gpio_regs * const gpio0 = (void *)GPIO0_BASE;
+
+   /* Turn on red LED, indicating full power mode */
+   spl_gpio_output(gpio0, GPIO(BANK_B, 5), 1);
+
+   return 0;
+}
+
+static int roc_pc_early_init_f(void)
 {
struct udevice *regulator;
int ret;
@@ -30,19 +40,11 @@ int board_early_init_f(void)
 out:
return 0;
 }
-#endif
-
-#if defined(CONFIG_TPL_BUILD)
-
-#define GPIO0_BASE  0xff72
 
 int board_early_init_f(void)
 {
-   struct rockchip_gpio_regs * const gpio0 = (void *)GPIO0_BASE;
-
-   /* Turn on red LED, indicating full power mode */
-   spl_gpio_output(gpio0, GPIO(BANK_B, 5), 1);
-
-   return 0;
+   if (IS_ENABLED(CONFIG_SPL_BUILD))
+   return led_setup();
+   else
+   return roc_pc_early_init_f();
 }
-#endif
diff --git a/configs/roc-pc-mezzanine-rk3399_defconfig 
b/configs/roc-pc-mezzanine-rk3399_defconfig
index c87a8568fc..15d511741f 100644
--- a/configs/roc-pc-mezzanine-rk3399_defconfig
+++ b/configs/roc-pc-mezzanine-rk3399_defconfig
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ARCH_ROCKCHIP=y
 CONFIG_SYS_TEXT_BASE=0x0020
+CONFIG_SPL_GPIO_SUPPORT=y
 CONFIG_ENV_SIZE=0x8000
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_ENV_SECT_SIZE=0x1000
@@ -21,7 +22,6 @@ CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_TPL=y
-CONFIG_TPL_GPIO_SUPPORT=y
 CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_MMC=y
diff --git a/configs/roc-pc-rk3399_defconfig b/configs/roc-pc-rk3399_defconfig
index 601f5c6ae1..2a6d0d22c8 100644
--- a/configs/roc-pc-rk3399_defconfig
+++ b/configs/roc-pc-rk3399_defconfig
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ARCH_ROCKCHIP=y
 CONFIG_SYS_TEXT_BASE=0x0020
+CONFIG_SPL_GPIO_SUPPORT=y
 CONFIG_ENV_SIZE=0x8000
 CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_ENV_SECT_SIZE=0x1000
@@ -21,7 +22,6 @@ CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_TPL=y
-CONFIG_TPL_GPIO_SUPPORT=y
 CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_MMC=y
-- 
2.25.1

[PATCH v5 4/5] rockchip: spl: Move board_early_init_f after cpu timer

[Jagan Teki]

Custom board_early_init_f not only deal with simple gpio
configuration but also have a possibility to access clocks
to process any clock related operations like checking reset
cause state and etc.

So, call it once the rockchip timer initialization done instead
of calling first place of board_init_f which doesn't have any
rockchip init code before.

This specific concern was tested with checking reset reason
via board_early_init_f, which indeed require a clk probe.

Signed-off-by: Jagan Teki 
---
Changes for v5:
- new patch

 arch/arm/mach-rockchip/spl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
index cddf4fd3d5..082828de66 100644
--- a/arch/arm/mach-rockchip/spl.c
+++ b/arch/arm/mach-rockchip/spl.c
@@ -122,8 +122,6 @@ void board_init_f(ulong dummy)
debug("\nspl:debug uart enabled in %s\n", __func__);
 #endif
 
-   board_early_init_f();
-
ret = spl_early_init();
if (ret) {
printf("spl_early_init() failed: %d\n", ret);
@@ -137,6 +135,9 @@ void board_init_f(ulong dummy)
/* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */
timer_init();
 #endif
+
+   board_early_init_f();
+
 #if !defined(CONFIG_TPL) || defined(CONFIG_SPL_RAM)
debug("\nspl:init dram\n");
ret = dram_init();
-- 
2.25.1

[PATCH v5 5/5] roc-rk3399-pc: Set LED only during POR and pwr_key=y

[Jagan Teki]

ROC-RK3399-PC has specific set of configurations for
on-board led setup.

Due to easiness for user to know the state of the board
roc-rk339-pc board code will setup the low power led
on/off, and waiting for user to press power key and then
glow full power led.

All this needs to happen only during power-on-reset not
for soft reset or WDT.

Also, it is not a proper usage to ask the user to press
the Power key if the board connected remotely, so add
the environment variable 'pwr_key' to check as well.

So, user need to press Power key only
- during POR
- pwr_key=y

Tested-by: Suniel Mahesh 
Signed-off-by: Jagan Teki 
---
Changes for v5:
- add changes on board file

 board/firefly/roc-pc-rk3399/roc-pc-rk3399.c | 35 -
 configs/roc-pc-mezzanine-rk3399_defconfig   |  3 ++
 configs/roc-pc-rk3399_defconfig |  3 ++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c 
b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
index 4db3dd739c..ff2dc028a1 100644
--- a/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
+++ b/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c
@@ -5,19 +5,52 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 
+#include 
 #include 
+#include 
 
+#define PMUGRF_BASE0xff32
 #define GPIO0_BASE 0xff72
 
+/**
+ * LED setup for roc-rk3399-pc
+ *
+ * 1. Set the low power leds (only during POR, pwr_key env is 'y')
+ *glow yellow LED, termed as low power
+ *poll for on board power key press
+ *once powe key pressed, turn off yellow
+ * 2. Turn on red LED, indicating full power mode
+ */
 static int led_setup(void)
 {
struct rockchip_gpio_regs * const gpio0 = (void *)GPIO0_BASE;
+   struct rk3399_pmugrf_regs * const pmugrf = (void *)PMUGRF_BASE;
+   bool press_pwr_key = false;
+
+   if (IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)) {
+   env_init();
+   env_load();
+   if (env_get_yesno("pwr_key") == 1)
+   press_pwr_key = true;
+   }
+
+   if (press_pwr_key && !strcmp(get_reset_cause(), "POR")) {
+   spl_gpio_output(gpio0, GPIO(BANK_A, 2), 1);
+
+   spl_gpio_set_pull(&pmugrf->gpio0_p, GPIO(BANK_A, 5),
+ GPIO_PULL_NORMAL);
+   while (readl(&gpio0->ext_port) & 0x20)
+   ;
+
+   spl_gpio_output(gpio0, GPIO(BANK_A, 2), 0);
+   }
 
-   /* Turn on red LED, indicating full power mode */
spl_gpio_output(gpio0, GPIO(BANK_B, 5), 1);
 
return 0;
diff --git a/configs/roc-pc-mezzanine-rk3399_defconfig 
b/configs/roc-pc-mezzanine-rk3399_defconfig
index 15d511741f..14cda5850e 100644
--- a/configs/roc-pc-mezzanine-rk3399_defconfig
+++ b/configs/roc-pc-mezzanine-rk3399_defconfig
@@ -8,6 +8,7 @@ CONFIG_ENV_SECT_SIZE=0x1000
 CONFIG_SPL_DM_SPI=y
 CONFIG_ROCKCHIP_RK3399=y
 CONFIG_TARGET_ROC_PC_RK3399=y
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x2
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_DEBUG_UART_BASE=0xFF1A
 CONFIG_DEBUG_UART_CLOCK=2400
@@ -20,6 +21,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
+CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_TPL=y
 CONFIG_CMD_BOOTZ=y
@@ -34,6 +36,7 @@ CONFIG_DEFAULT_DEVICE_TREE="rk3399-roc-pc-mezzanine"
 CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names 
interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_ROCKCHIP_GPIO=y
 CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_MISC=y
diff --git a/configs/roc-pc-rk3399_defconfig b/configs/roc-pc-rk3399_defconfig
index 2a6d0d22c8..85f5c8f86b 100644
--- a/configs/roc-pc-rk3399_defconfig
+++ b/configs/roc-pc-rk3399_defconfig
@@ -8,6 +8,7 @@ CONFIG_ENV_SECT_SIZE=0x1000
 CONFIG_SPL_DM_SPI=y
 CONFIG_ROCKCHIP_RK3399=y
 CONFIG_TARGET_ROC_PC_RK3399=y
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x2
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_DEBUG_UART_BASE=0xFF1A
 CONFIG_DEBUG_UART_CLOCK=2400
@@ -20,6 +21,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
+CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_TPL=y
 CONFIG_CMD_BOOTZ=y
@@ -33,6 +35,7 @@ CONFIG_DEFAULT_DEVICE_TREE="rk3399-roc-pc"
 CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names 
interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_ROCKCHIP_GPIO=y
 CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_MISC=y
-- 
2.25.1

[PATCH v5 3/5] rockchip: Separate the reset cause from display cpuinfo

[Jagan Teki]

reset cause is a generic functionality based on the soc
cru registers in rockchip. This can be used for printing
the cause of reset in cpuinfo or some other place where
reset cause is needed. 

Other than cpuinfo, reset cause can also be using during
bootcount for checking the specific reset cause and glow
the led based on the reset cause.

So, let's separate the reset cause code from cpuinfo, and
add a check to build it for rk3399, rk3288 since these two
soc are supporting reset cause as of now.

Tested-by: Suniel Mahesh 
Signed-off-by: Jagan Teki 
---
Changes for v5:
- update Makefile

 arch/arm/include/asm/arch-rockchip/cru.h |  2 ++
 arch/arm/mach-rockchip/Makefile  |  5 -
 arch/arm/mach-rockchip/cpu-info.c| 20 
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/cru.h 
b/arch/arm/include/asm/arch-rockchip/cru.h
index d2057cb738..13ea4aba8e 100644
--- a/arch/arm/include/asm/arch-rockchip/cru.h
+++ b/arch/arm/include/asm/arch-rockchip/cru.h
@@ -30,4 +30,6 @@ enum {
 
 #define MHz100
 
+char *get_reset_cause(void);
+
 #endif /* _ROCKCHIP_CLOCK_H */
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index 5b38526fe0..121f23a563 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -22,11 +22,14 @@ ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
 # we can have the preprocessor correctly recognise both 0x0 and 0
 # meaning "turn it off".
 obj-y += boot_mode.o
-obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
 obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
 obj-$(CONFIG_MISC_INIT_R) += misc.o
 endif
 
+ifeq ($(CONFIG_TPL_BUILD),)
+obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
+endif
+
 obj-$(CONFIG_$(SPL_TPL_)RAM) += sdram.o
 
 obj-$(CONFIG_ROCKCHIP_PX30) += px30/
diff --git a/arch/arm/mach-rockchip/cpu-info.c 
b/arch/arm/mach-rockchip/cpu-info.c
index bb5a198039..d0f030109f 100644
--- a/arch/arm/mach-rockchip/cpu-info.c
+++ b/arch/arm/mach-rockchip/cpu-info.c
@@ -13,7 +13,7 @@
 #include 
 #include 
 
-static char *get_reset_cause(void)
+char *get_reset_cause(void)
 {
struct rockchip_cru *cru = rockchip_get_cru();
char *cause = NULL;
@@ -41,21 +41,25 @@ static char *get_reset_cause(void)
cause = "unknown reset";
}
 
-   /**
-* reset_reason env is used by rk3288, due to special use case
-* to figure it the boot behavior. so keep this as it is.
-*/
-   env_set("reset_reason", cause);
-
return cause;
 }
 
+#if CONFIG_IS_ENABLED(DISPLAY_CPUINFO)
 int print_cpuinfo(void)
 {
+   char *cause = get_reset_cause();
+
printf("SoC: Rockchip %s\n", CONFIG_SYS_SOC);
-   printf("Reset cause: %s\n", get_reset_cause());
+   printf("Reset cause: %s\n", cause);
+
+   /**
+* reset_reason env is used by rk3288, due to special use case
+* to figure it the boot behavior. so keep this as it is.
+*/
+   env_set("reset_reason", cause);
 
/* TODO print operating temparature and clock */
 
return 0;
 }
+#endif
-- 
2.25.1

[PATCH v5 2/5] rockchip: Don't clear the reset status reg

[Jagan Teki]

reset reason can be used several stages of U-Boot bootloader
like SPL, U-Boot proper based on the requirements.

Clearing the status register end of get_reset_cause will end
up showing the wrong reset cause when it read the second time.
For example, if board resets, SPL reads the reset status as
RST whereas U-Boot proper reads the status as POR.

However, based on the latest testing clearing reset status
won't be required for determine the last reset cause or
following resets.

Signed-off-by: Jagan Teki 
---
Changes for v5:
- new patch

 arch/arm/include/asm/arch-rockchip/cru.h | 1 -
 arch/arm/mach-rockchip/cpu-info.c| 6 --
 2 files changed, 7 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/cru.h 
b/arch/arm/include/asm/arch-rockchip/cru.h
index 5eb17f9d55..d2057cb738 100644
--- a/arch/arm/include/asm/arch-rockchip/cru.h
+++ b/arch/arm/include/asm/arch-rockchip/cru.h
@@ -26,7 +26,6 @@ enum {
SND_GLB_TSADC_RST_ST= BIT(3),
FST_GLB_WDT_RST_ST  = BIT(4),
SND_GLB_WDT_RST_ST  = BIT(5),
-   GLB_RST_ST_MASK = GENMASK(5, 0),
 };
 
 #define MHz100
diff --git a/arch/arm/mach-rockchip/cpu-info.c 
b/arch/arm/mach-rockchip/cpu-info.c
index 21ca9dedce..bb5a198039 100644
--- a/arch/arm/mach-rockchip/cpu-info.c
+++ b/arch/arm/mach-rockchip/cpu-info.c
@@ -47,12 +47,6 @@ static char *get_reset_cause(void)
 */
env_set("reset_reason", cause);
 
-   /*
-* Clear glb_rst_st, so we can determine the last reset cause
-* for following resets.
-*/
-   rk_clrreg(&cru->glb_rst_st, GLB_RST_ST_MASK);
-
return cause;
 }
 
-- 
2.25.1

Re: [GIT PULL] please pull NXP i.MX

[Stefano Babic]

Hi Peng,

On 14.07.20 09:36, Peng Fan wrote:
> Hi Stefano,
> 
> This is collect of patches that pending in patchwork and some new patches 
> posted recently.
> The PR is rebased on Tom's master branch since imx/master was not up to date, 
> should I do the PR based on imx/master?
> CI: https://travis-ci.org/github/MrVan/u-boot/builds/707889918
> 
> 
> i.MX DDR driver fix/update for i.MX8M
> i.MX pinctrl driver fix.
> Use arm_smccc_smc to remove imx sip function
> i.MX8M clk update
> support booting aarch32 kernel on aarch64 hardware
> fused part support for i.MX8MP
> 
> 

Applied to -master, thanks !

Regards,
Stefano

> Thanks,
> Peng.
> 
> The following changes since commit 497c7598c4e713eb9ad88fd7963e57b21b8b35e1:
> 
>   Merge branch 'master' of 
> https://gitlab.denx.de/u-boot/custodians/u-boot-spi (2020-07-11 17:40:00 
> -0400)
> 
> are available in the Git repository at:
> 
>   https://github.com/MrVan/u-boot.git tags/nxp-imx-7-14-2020
> 
> for you to fetch changes up to 86e9d7e8143eab4cfe4b74628153af9006bb1ad2:
> 
>   imx8mm_evk: enlarge CONFIG_SYS_BOOTM_LEN (2020-07-14 15:23:48 +0800)
> 
> 
> Jacky Bai (2):
>   driver: ddr: imx: skip ddr_ss_gpr config on imx8mn
>   driver: ddr: imx: correct the pwrctl setting of selfref_en on imx8m
> 
> Jian Li (3):
>   imx8mp: enable rd_port_urgent
>   imx8mp: DDR performance tunning
>   imx8mp: Disables use of MR4 TUF flag (MR4[7]) bit
> 
> Oliver Chen (1):
>   drivers: ddr: imx Workaround for i.MX8M DDRPHY rank to rank issue
> 
> Peng Fan (22):
>   pinctrl: imx7: move soc info to data section
>   pinctrl: imx8m: move soc info to data section
>   pinctrl: imx5: move soc info to data section
>   imx8: misc: use arm_smccc_smc
>   imx8m: soc: use arm_smccc_smc
>   imx: bootaux: use arm_smccc_smc
>   imx8: fuse: use arm_smccc_smc
>   imx: power-domain: use arm_smccc_smc
>   imx: remove imx sip file
>   clk: imx8mm: fix clk set parent
>   clk: imx8mm: Add qspi clock
>   imx8m: configure arm clk sources from PLL
>   imx8m: configure NoC clk
>   imx8m: add sdhc/nand/ecspi clk api
>   imx8m: add eqos clk
>   imx8m: workaround ROM serror
>   imx8m: power down fused cores
>   imx8m: disable nodes before kernel/mfgtool boot for fused part
>   clk: imx8m: drop clk settings
>   imx8m: Refactor the OPTEE memory removal
>   imx8m: implement armv8_el2_to_aarch32
>   imx8mm_evk: enlarge CONFIG_SYS_BOOTM_LEN
> 
> Sherry Sun (1):
>   drivers: ddr: imx8mp: Add inline ECC feature support
> 
> Ye Li (5):
>   clk: clk-imx8mn: Update clock tree and support set parent
>   clk: imx8mm/8mn: Add USB clocks
>   clk: imx8mp: Update imx8mp ccf clock driver
>   imx8mp: Add fused parts support
>   imx8mn/imx8mp: override env_get_offset and env_get_location
> 
>  arch/arm/include/asm/arch-imx/cpu.h|   5 ++
>  arch/arm/include/asm/arch-imx8m/ddr.h  |  10 +++
>  arch/arm/include/asm/arch-imx8m/imx-regs.h | 158 
> +
>  arch/arm/include/asm/mach-imx/sys_proto.h  |   9 +-
>  arch/arm/mach-imx/Makefile |   2 +-
>  arch/arm/mach-imx/cpu.c|  12 ++-
>  arch/arm/mach-imx/imx8/misc.c  |   7 +-
>  arch/arm/mach-imx/imx8m/clock_imx8mm.c | 299 
> -
>  arch/arm/mach-imx/imx8m/lowlevel_init.S|  12 +++
>  arch/arm/mach-imx/imx8m/soc.c  | 612 
> ++---
>  arch/arm/mach-imx/imx_bootaux.c|  11 ++-
>  arch/arm/mach-imx/sip.c|  48 --
>  board/beacon/imx8mm/imx8mm_beacon.c|  11 ---
>  board/freescale/imx8mm_evk/imx8mm_evk.c|  11 ---
>  board/freescale/imx8mn_evk/imx8mn_evk.c|   7 --
>  board/freescale/imx8mp_evk/imx8mp_evk.c|  40 -
>  board/freescale/imx8mp_evk/lpddr4_timing.c |   5 +-
>  board/freescale/imx8mq_evk/imx8mq_evk.c|  11 ---
>  board/google/imx8mq_phanbell/imx8mq_phanbell.c |  11 ---
>  board/technexion/pico-imx8mq/pico-imx8mq.c |  26 +++---
>  board/toradex/verdin-imx8mm/verdin-imx8mm.c|  11 ---
>  configs/imx8mp_evk_defconfig   |   1 -
>  drivers/clk/imx/clk-imx8mm.c   |  63 ++---
>  drivers/clk/imx/clk-imx8mn.c   | 108 +++---
>  drivers/clk/imx/clk-imx8mp.c   |  52 +++
>  drivers/ddr/imx/imx8m/Kconfig  |   7 ++
>  drivers/ddr/imx/imx8m/ddr_init.c   |  79 -
>  drivers/ddr/imx/imx8m/ddrphy_train.c   |   7 ++
>  drivers/ddr/imx/imx8m/ddr

RE: [PATCH v1 3/4] clk: agilex: Handle clock configuration differently in SPL and U-Boot proper

[Tan, Ley Foon]

> -Original Message-
> From: Ang, Chee Hong 
> Sent: Friday, July 10, 2020 8:55 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut ; Simon Goldschmidt
> ; See, Chin Liang
> ; Tan, Ley Foon ; Ang,
> Chee Hong 
> Subject: [PATCH v1 3/4] clk: agilex: Handle clock configuration differently in
> SPL and U-Boot proper
> 
> Since warm reset may optionally set the CLock Manager to'boot mode', the
> clock driver should always force the Agilex's Clock Manager to 'boot mode'
> before the clock driver start configuring the Clock Manager in SPL.
> In SSBL, clock driver will skip the Clock Manager configuration if it's 
> already
> being setup by SPL (Clock Manager NOT in 'boot
> mode') to prevent any inaccurate clocking issues happened on HPS
> peripherals such as UART, MAC and etc.
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  drivers/clk/altera/clk-agilex.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk-agilex.c
> index b5cf187364..c83eb2efb9 100644
> --- a/drivers/clk/altera/clk-agilex.c
> +++ b/drivers/clk/altera/clk-agilex.c
> @@ -171,6 +171,16 @@ static void clk_basic_init(struct udevice *dev,
>   if (!cfg)
>   return;
> 
> +#ifdef CONFIG_SPL_BUILD
> + /* Always force clock manager into boot mode before any
> configuration */
> + clk_write_ctrl(plat,
> +CM_REG_READL(plat, CLKMGR_CTRL) |
> CLKMGR_CTRL_BOOTMODE); #else
"#else" is at the end of line, is this patch display issue or coding issue?


Regards
Ley Foon

RE: [PATCH v1 4/4] clk: agilex: Additional membus writes for HPS PLL

[Tan, Ley Foon]

> -Original Message-
> From: Ang, Chee Hong 
> Sent: Friday, July 10, 2020 8:55 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut ; Simon Goldschmidt
> ; See, Chin Liang
> ; Tan, Ley Foon ; Ang,
> Chee Hong 
> Subject: [PATCH v1 4/4] clk: agilex: Additional membus writes for HPS PLL
> 
> Add additional membus writes to configure main and peripheral PLL for
> Agilex's clock manager.
> 
> Signed-off-by: Chee Hong Ang 
> ---
Reviewed-by: Ley Foon Tan 

RE: [PATCH] arm: socfpga: soc64: Remove PHY interface setup from misc arch init

[Tan, Ley Foon]

> -Original Message-
> From: Ang, Chee Hong 
> Sent: Friday, July 10, 2020 11:53 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut ; Simon Goldschmidt
> ; See, Chin Liang
> ; Tan, Ley Foon ; Ang,
> Chee Hong 
> Subject: [PATCH] arm: socfpga: soc64: Remove PHY interface setup from
> misc arch init
> 
> 'dwmac_socfpga' driver will setup the PHY interface during probe.
> PHY interface setup in arch_misc_init() is no longer needed.
> 
> Signed-off-by: Chee Hong Ang 
> ---

Reviewed-by: Ley Foon Tan 

RE: [PATCH] arm: socfpga: soc64: Initialize timer in SPL only

[Tan, Ley Foon]

> -Original Message-
> From: Ang, Chee Hong 
> Sent: Friday, July 10, 2020 11:53 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut ; Simon Goldschmidt
> ; See, Chin Liang
> ; Tan, Ley Foon ; Ang,
> Chee Hong 
> Subject: [PATCH] arm: socfpga: soc64: Initialize timer in SPL only
> 
> Timer only need to be initialized once in SPL.
> This patch remove the redundancy of initializing the timer again in U-Boot
> proper
> 
> Signed-off-by: Chee Hong Ang 
> ---

Reviewed-by: Ley Foon Tan 

[PATCH 1/1] efi_loader: restructure code for TEE variables

[Heinrich Schuchardt]

When using secure boot functions needed both for file and TEE based UEFI
variables have to be moved to the common code module efi_var_common.c.

Signed-off-by: Heinrich Schuchardt 
---
 include/efi_variable.h  |   7 ++
 lib/efi_loader/efi_var_common.c | 155 +++
 lib/efi_loader/efi_variable.c   | 159 
 3 files changed, 162 insertions(+), 159 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index bc5985cfdb..5eec407a2b 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -195,4 +195,11 @@ efi_status_t efi_var_mem_ins(u16 *variable_name,
  */
 u64 efi_var_mem_free(void);

+/**
+ * efi_init_secure_state - initialize secure boot state
+ *
+ * Return: status code
+ */
+efi_status_t efi_init_secure_state(void);
+
 #endif
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 1e2be1135b..36e31b4d45 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -9,6 +9,16 @@
 #include 
 #include 

+enum efi_secure_mode {
+   EFI_MODE_SETUP,
+   EFI_MODE_USER,
+   EFI_MODE_AUDIT,
+   EFI_MODE_DEPLOYED,
+};
+
+static bool efi_secure_boot;
+static enum efi_secure_mode efi_secure_mode;
+
 /**
  * efi_efi_get_variable() - retrieve value of a UEFI variable
  *
@@ -138,3 +148,148 @@ efi_status_t EFIAPI efi_query_variable_info(

return EFI_EXIT(ret);
 }
+
+/**
+ * efi_set_secure_state - modify secure boot state variables
+ * @secure_boot:   value of SecureBoot
+ * @setup_mode:value of SetupMode
+ * @audit_mode:value of AuditMode
+ * @deployed_mode: value of DeployedMode
+ *
+ * Modify secure boot status related variables as indicated.
+ *
+ * Return: status code
+ */
+static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode,
+u8 audit_mode, u8 deployed_mode)
+{
+   efi_status_t ret;
+   const u32 attributes_ro = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS |
+ EFI_VARIABLE_READ_ONLY;
+   const u32 attributes_rw = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS;
+
+   efi_secure_boot = secure_boot;
+
+   ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
+  attributes_ro, sizeof(secure_boot),
+  &secure_boot, false);
+   if (ret != EFI_SUCCESS)
+   goto err;
+
+   ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid,
+  attributes_ro, sizeof(setup_mode),
+  &setup_mode, false);
+   if (ret != EFI_SUCCESS)
+   goto err;
+
+   ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid,
+  audit_mode || setup_mode ?
+  attributes_ro : attributes_rw,
+  sizeof(audit_mode), &audit_mode, false);
+   if (ret != EFI_SUCCESS)
+   goto err;
+
+   ret = efi_set_variable_int(L"DeployedMode",
+  &efi_global_variable_guid,
+  audit_mode || deployed_mode || setup_mode ?
+  attributes_ro : attributes_rw,
+  sizeof(deployed_mode), &deployed_mode,
+  false);
+err:
+   return ret;
+}
+
+/**
+ * efi_transfer_secure_state - handle a secure boot state transition
+ * @mode:  new state
+ *
+ * Depending on @mode, secure boot related variables are updated.
+ * Those variables are *read-only* for users, efi_set_variable_int()
+ * is called here.
+ *
+ * Return: status code
+ */
+static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
+{
+   efi_status_t ret;
+
+   EFI_PRINT("Switching secure state from %d to %d\n", efi_secure_mode,
+ mode);
+
+   if (mode == EFI_MODE_DEPLOYED) {
+   ret = efi_set_secure_state(1, 0, 0, 1);
+   if (ret != EFI_SUCCESS)
+   goto err;
+   } else if (mode == EFI_MODE_AUDIT) {
+   ret = efi_set_variable_int(L"PK", &efi_global_variable_guid,
+  EFI_VARIABLE_BOOTSERVICE_ACCESS |
+  EFI_VARIABLE_RUNTIME_ACCESS,
+  0, NULL, false);
+   if (ret != EFI_SUCCESS)
+   goto err;
+
+   ret = efi_set_secure_state(0, 1, 1, 0);
+   if (ret != EFI_SUCCESS)
+   goto err;
+   } else if (mode == EFI_MODE_USER) {
+   ret = efi_set_secure_state(1, 0, 0, 0);
+   if (ret != EFI_SUCCESS)
+ 

[PATCH 1/1] efi_loader: update secure state

[Heinrich Schuchardt]

Update the UEFI secure state when variable 'PK' is updated in the TEE
variables implementation.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_variable_tee.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/efi_loader/efi_variable_tee.c 
b/lib/efi_loader/efi_variable_tee.c
index 24e0663ebd..1046e0d470 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -557,6 +557,10 @@ efi_status_t efi_set_variable_int(u16 *variable_name, 
const efi_guid_t *vendor,
var_property.maxsize = var_acc->data_size;
ret = set_property_int(variable_name, name_size, vendor, 
&var_property);
}
+
+   if (alt_ret == EFI_SUCCESS ret == EFI_SUCCESS &&
+   !u16_strcmp(variable_name, L"PK"))
+   ret = efi_init_secure_state();
 out:
free(comm_buf);
return alt_ret == EFI_SUCCESS ? ret : alt_ret;
@@ -716,5 +720,9 @@ efi_status_t efi_init_variables(void)
  MM_VARIABLE_COMMUNICATE_SIZE +
  max_payload_size;

+   ret = efi_init_secure_state();
+   if (ret != EFI_SUCCESS)
+   return ret;
+
return EFI_SUCCESS;
 }
--
2.27.0

Re: [PATCH] memsize: Make get_ram_size() work with arbitary RAM size

[Bin Meng]

Hi Wolfgang,

On Tue, Jul 14, 2020 at 1:18 AM Wolfgang Denk  wrote:
>
> Dear Bin Meng,
>
> In message <1594378641-26360-1-git-send-email-bmeng...@gmail.com> you wrote:
> >
> > Currently get_ram_size() only works with certain RAM size like 1GiB,
> > 2GiB, 4GiB, 8GiB, etc. Chanage the codes to work with any RAM size.
>
> I'm afraid I don't understand this change,  Can you please explain a
> bit more detailed what "any RAM size" means?

I meant "any RAM size" that is not power of two.

>
> The existing code should work fine with any RAM size that is a power
> of two (and the algoithm used is based on this assumption, so the
> code would fail if it is not met).

Unfortunately this limitation is not documented clearly.

>
> > - long   save[BITS_PER_LONG - 1];
> > - long   save_base;
> > - long   cnt;
> > - long   val;
> > - long   size;
> > - inti = 0;
> > + long save[BITS_PER_LONG - 1];
> > + long save_base;
> > + long cnt;
> > + long val;
> > + long size;
> > + int i = 0;
>
> Why do you change the formatting here?  I can see no need for that?

I see the above are the only places in this file that do not follow
our coding practices. Since I was modifying this function, I think
it's fine to do the updates while we are here.

>
>
> > + long n = maxsize / sizeof(long);
> >
> > - for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) {
> > + n = __ffs(n);
> > + n = BIT(n);
> > +
> > + for (cnt = n >> 1; cnt > 0; cnt >>= 1) {
>
> I can only speculate - but do you think that this will work for a
> memory size of - for example - 2.5 GiB as might result from combining
> two banks of 2 GiB resp. 512 MiB ?   I bet it doesn't.

Correct. This issue was exposed when I was testing U-Boot with QEMU on
RISC-V. With QEMU we can instantiate arbitrary RAM size for a given
machine.

>
> For correct operation (as originally intended) you would always
> specify a maxsize twice as large as the maximum possible memory size
> of a bank of memory, and the function would return the real size it
> found.

I don't think this function can work as expected. Even if I pass a
power-of-two RAM size to this function, it could still fail. Imagine
the target has 2GiB memory size, and I pass 8GiB as the maxsize to
this function. The function will hang when it tries to read/write
something at an address that is beyond 2GiB.

>
> Any other use, especially not checking one bank of memory at a time,
> will not work as intended.  And I have yet to see systems where
> the size of a bank of memory is not a power of two.
>
> So I feel what you are doing here is not an improvement, but a
> "workaround" for some incorrect usage.
>

Given what you mentioned the function limitation, we should update the
codes with the above power of two assumptions documented.

> I don't think we should accept this patch.
>

Regards,
Bin

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Simon South]

Peter Robinson  writes:
> It should be fixed in the main devel repo, commit 3a5771249

That commit enables the OHCI driver but doesn't lead to a working
keyboard, at least for me.

Have you actually tested this successfully on a PBP? (Has anyone else?)
I wonder if there's something different about my unit.

-- 
Simon South
si...@simonsouth.net

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Peter Robinson]

On Tue, Jul 14, 2020 at 11:50 AM Simon South  wrote:
>
> Peter Robinson  writes:
> > It should be fixed in the main devel repo, commit 3a5771249
>
> That commit enables the OHCI driver but doesn't lead to a working
> keyboard, at least for me.
>
> Have you actually tested this successfully on a PBP? (Has anyone else?)
> I wonder if there's something different about my unit.

It was working when I sent the patches.

[PATCH 1/1] doc: provide links to Microsoft UEFI certificates

[Heinrich Schuchardt]

Some distributions provide UEFI binaries like Shim that have been signed
using a Microsoft certificate. Provide the download paths for the public
keys.

Signed-off-by: Heinrich Schuchardt 
---
 doc/uefi/uefi.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index 03d6fd0c6a..a72e729cc8 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -188,6 +188,15 @@ on the sandbox
 cd 
 pytest.py test/py/tests/test_efi_secboot/test_signed.py --bd sandbox

+UEFI binaries may be signed by Microsoft using the following certificates:
+
+* KEK: Microsoft Corporation KEK CA 2011
+  http://go.microsoft.com/fwlink/?LinkId=321185.
+* db: Microsoft Windows Production PCA 2011
+  http://go.microsoft.com/fwlink/p/?linkid=321192.
+* db: Microsoft Corporation UEFI CA 2011
+  http://go.microsoft.com/fwlink/p/?linkid=321194.
+
 Using OP-TEE for EFI variables
 ~~

--
2.27.0

Re: [PATCH 1/1] efi_loader: update secure state

[ilias . apalodimas]

On Tue, Jul 14, 2020 at 12:06:56PM +0200, Heinrich Schuchardt wrote:
> Update the UEFI secure state when variable 'PK' is updated in the TEE
> variables implementation.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/efi_loader/efi_variable_tee.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_variable_tee.c 
> b/lib/efi_loader/efi_variable_tee.c
> index 24e0663ebd..1046e0d470 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -557,6 +557,10 @@ efi_status_t efi_set_variable_int(u16 *variable_name, 
> const efi_guid_t *vendor,
>   var_property.maxsize = var_acc->data_size;
>   ret = set_property_int(variable_name, name_size, vendor, 
> &var_property);
>   }
> +
> + if (alt_ret == EFI_SUCCESS ret == EFI_SUCCESS &&
> + !u16_strcmp(variable_name, L"PK"))
> + ret = efi_init_secure_state();

There's an && missing here. In any case don't check for both alt_ret and ret,
just goto out; if the above set_property_int() fails and you should be able to 
check for 'ret' only.

>  out:
>   free(comm_buf);
>   return alt_ret == EFI_SUCCESS ? ret : alt_ret;
> @@ -716,5 +720,9 @@ efi_status_t efi_init_variables(void)
> MM_VARIABLE_COMMUNICATE_SIZE +
> max_payload_size;
> 
> + ret = efi_init_secure_state();
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
>   return EFI_SUCCESS;
>  }
> --
> 2.27.0
> 

Thanks
/Ilias

Re: [PATCH 1/1] efi_loader: update secure state

[Ilias Apalodimas]

On Tue, 14 Jul 2020 at 14:14,  wrote:
>
> On Tue, Jul 14, 2020 at 12:06:56PM +0200, Heinrich Schuchardt wrote:
> > Update the UEFI secure state when variable 'PK' is updated in the TEE
> > variables implementation.
> >
> > Signed-off-by: Heinrich Schuchardt 
> > ---
> >  lib/efi_loader/efi_variable_tee.c | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_variable_tee.c 
> > b/lib/efi_loader/efi_variable_tee.c
> > index 24e0663ebd..1046e0d470 100644
> > --- a/lib/efi_loader/efi_variable_tee.c
> > +++ b/lib/efi_loader/efi_variable_tee.c
> > @@ -557,6 +557,10 @@ efi_status_t efi_set_variable_int(u16 *variable_name, 
> > const efi_guid_t *vendor,
> >   var_property.maxsize = var_acc->data_size;
> >   ret = set_property_int(variable_name, name_size, vendor, 
> > &var_property);
> >   }
> > +
> > + if (alt_ret == EFI_SUCCESS ret == EFI_SUCCESS &&
> > + !u16_strcmp(variable_name, L"PK"))
> > + ret = efi_init_secure_state();
>
> There's an && missing here. In any case don't check for both alt_ret and ret,
> just goto out; if the above set_property_int() fails and you should be able to
> check for 'ret' only.

Actually if you add the goto out, no check at all is required, just your strcmp

>
> >  out:
> >   free(comm_buf);
> >   return alt_ret == EFI_SUCCESS ? ret : alt_ret;
> > @@ -716,5 +720,9 @@ efi_status_t efi_init_variables(void)
> > MM_VARIABLE_COMMUNICATE_SIZE +
> > max_payload_size;
> >
> > + ret = efi_init_secure_state();
> > + if (ret != EFI_SUCCESS)
> > + return ret;
> > +
> >   return EFI_SUCCESS;
> >  }
> > --
> > 2.27.0
> >
>
> Thanks
> /Ilias

RE: [PATCH v1 3/4] clk: agilex: Handle clock configuration differently in SPL and U-Boot proper

[Ang, Chee Hong]

> > From: Ang, Chee Hong 
> > Sent: Friday, July 10, 2020 8:55 PM
> > To: u-boot@lists.denx.de
> > Cc: Marek Vasut ; Simon Goldschmidt
> > ; See, Chin Liang
> > ; Tan, Ley Foon ;
> > Ang, Chee Hong 
> > Subject: [PATCH v1 3/4] clk: agilex: Handle clock configuration
> > differently in SPL and U-Boot proper
> >
> > Since warm reset may optionally set the CLock Manager to'boot mode',
> > the clock driver should always force the Agilex's Clock Manager to 'boot
> mode'
> > before the clock driver start configuring the Clock Manager in SPL.
> > In SSBL, clock driver will skip the Clock Manager configuration if
> > it's already being setup by SPL (Clock Manager NOT in 'boot
> > mode') to prevent any inaccurate clocking issues happened on HPS
> > peripherals such as UART, MAC and etc.
> >
> > Signed-off-by: Chee Hong Ang 
> > ---
> >  drivers/clk/altera/clk-agilex.c | 10 ++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/clk/altera/clk-agilex.c
> > b/drivers/clk/altera/clk-agilex.c index b5cf187364..c83eb2efb9 100644
> > --- a/drivers/clk/altera/clk-agilex.c
> > +++ b/drivers/clk/altera/clk-agilex.c
> > @@ -171,6 +171,16 @@ static void clk_basic_init(struct udevice *dev,
> > if (!cfg)
> > return;
> >
> > +#ifdef CONFIG_SPL_BUILD
> > +   /* Always force clock manager into boot mode before any
> > configuration */
> > +   clk_write_ctrl(plat,
> > +  CM_REG_READL(plat, CLKMGR_CTRL) |
> > CLKMGR_CTRL_BOOTMODE); #else
> "#else" is at the end of line, is this patch display issue or coding issue?
Only happen in display. Code is fine.
> 
> 
> Regards
> Ley Foon

Re: [PATCH] memsize: Make get_ram_size() work with arbitary RAM size

[Wolfgang Denk]

Dear Bin Meng,

In message  
you wrote:
>
> > I'm afraid I don't understand this change,  Can you please explain a
> > bit more detailed what "any RAM size" means?
>
> I meant "any RAM size" that is not power of two.

I was afraid you meant this.

> > The existing code should work fine with any RAM size that is a power
> > of two (and the algoithm used is based on this assumption, so the
> > code would fail if it is not met).
>
> Unfortunately this limitation is not documented clearly.

You are right - the code is completely undocumented, not even the
fact that it should always operate on a single bank of memory at a
time.

> > For correct operation (as originally intended) you would always
> > specify a maxsize twice as large as the maximum possible memory size
> > of a bank of memory, and the function would return the real size it
> > found.
>
> I don't think this function can work as expected. Even if I pass a
> power-of-two RAM size to this function, it could still fail. Imagine
> the target has 2GiB memory size, and I pass 8GiB as the maxsize to
> this function. The function will hang when it tries to read/write
> something at an address that is beyond 2GiB.

It should not hang if used properly - but of course this depends a
lot on the hardware properties, and you have to know what you are
doing.

When the code was written, all we had to deal with was Power
Architecture systems with pretty flexible and easy to ptogram memory
controllers.  A typical use case was a system where you could for
example populate either 64 or 128 or 256 MB of RAM.  In such a case
you would run the code with a max size of 512 MB, after configuring
he memory controller to map such a size.  In this case, acceses to
the whole 512 MB range will work without problems, the higher
address ranges just mirroring the data of the actually populated
RAM.  This mirroring will be detected, so you get the information
how big the populated RAM really is.

If we would only check with a max size of 256 MB, we would not be
able to detect the case when there is bigger RAM (say, 512 MB)
populated.

I am aware that there are situations where you can't program the
memory controller so freely, so we often cannot use this complete
testing and lose the ability to detect working, but bigger RAM.

In any case we always test only the memory locations at the
power-of-two borders - the main reason is that this test is supposed
to run on every boot, so it must be fast.

In your case this immediately shows the problem.  Assume you have a
memory configuration which is supposed to have 2 GiB + 512 MiB,
but by mistake wrong components were fit and instead of 512 MiB
there are only 128 MiB actually present.

U-Boot will test memory below the ... 128M, 256M, 512M, 1G, 2G (and
ideally 4G) limits.  It will tell you that there are 2 GiB working
memory.  It will NOT test any location between 2 GB and 4GB, so it
has no chance to find out if there is 128 MB or 512 MB present, or
nothing at all, or if this memory is not working at all and
returning random data.

This code works only for bank sizes that are a power of two!

> Given what you mentioned the function limitation, we should update the
> codes with the above power of two assumptions documented.

That would indeed make sense.  On the other hand, I would have
expected that this behaviour is kind of obvious from the fact, that
we just bit shift the address location used for testing?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
The number you have dialed is imaginary. Please divide by 0  and  try
again.

Re: Pull request for UEFI sub-system for efi-2020-10-rc1 (3)

[Tom Rini]

On Mon, Jul 13, 2020 at 12:42:55PM +0200, Heinrich Schuchardt wrote:

> The following changes since commit 4a9146c29573dbfa661918280d9522a01f6ca919:
> 
>   Merge tag 'dm-pull-10jul20' of
> https://gitlab.denx.de/u-boot/custodians/u-boot-dm (2020-07-10 16:22:57
> -0400)
> 
> are available in the Git repository at:
> 
>   https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git
> tags/efi-2020-10-rc1-3
> 
> for you to fetch changes up to 4a3155de3dbadfcb933287dbb84c8eff0fd951eb:
> 
>   efi_selftest: adjust runtime test for variables (2020-07-11 23:14:17
> +0200)
> 
> Testing in Travis CI and Gitlab CI showed not problems:
> 
> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/pipelines/4006
> https://travis-ci.org/github/xypron2/u-boot/builds/707245196
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: Using gerrit or github for review?

[Tom Rini]

On Tue, Jul 14, 2020 at 08:09:23AM +0200, Michal Simek wrote:
> 
> 
> On 13. 07. 20 22:36, Tom Rini wrote:
> > On Mon, Jul 13, 2020 at 12:25:42PM -0600, Simon Glass wrote:
> > 
> >> Hi,
> >>
> >> At present U-Boot uses the mailing list for patch review. What do
> >> people think about trying out geritt or github for this? I'd be
> >> willing to do a trial with the -dm mailing list.
> >>
> >> My idea is that patman would email out the patches and also upload
> >> them to one of these systems. With geritt, emails are sent every time
> >> there is a review, but for github I'm not sure.
> > 
> > As you said, you also intended to include gitlab in the list.
> > 
> > So, things that I think could be better, or more widely known, would be
> > how to trigger Azure CI runs (as there's good resources available for
> > free) and GitLab for non-custodians.  By which I mean, anyone can get a
> > CI run on Azure today via a GitHub PR.  Making that known more widely
> > would be good.
> > 
> > For patch review, are there any of the Linux kernel bots that are well
> > enough documented for someone else to pick up and use?  I feel like our
> > biggest problems are:
> > - Lack of reviewers in general for various areas.
> > - Lack of feedback to users (developers) before being picked up for
> >   general changes.
> > 
> > For the second problem, I feel like some of the Linux bots would be a
> > little helpful, but probably require some tweaking (figure out when /
> > how to fire off more limited CI, or just do a daily CI run vs every
> > patch, given the size of our CI build).
> > 
> 
> Don't have details but patchwork should support (somehow) to run some
> checks.
> Here you can see it working.
> http://patchwork.ozlabs.org/project/devicetree-bindings/patch/1594676120-5862-7-git-send-email-prabhakar.mahadev-lad...@bp.renesas.com/
> or
> http://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200713132927.24925-2-abai...@baylibre.com/
> 
> It means when the patch reach patchwork checks can run. Would be
> wonderful to get PASS/FAIL with link via email from CI loop.
> 
> I don't have experience with gerrit but definitely don't like using
> github for contribution and prefer to use review based on emails.

Oh wow, that is useful.  Rob, can you share what you're doing for that?
Thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 17/44] sound: Add an ACPI driver for Maxim MAX98357ac

[Simon Glass]

Hi Bin,

On Mon, 13 Jul 2020 at 23:01, Bin Meng  wrote:
>
> Hi Simon,
>
> On Tue, Jul 14, 2020 at 9:28 AM Bin Meng  wrote:
> >
> > Hi Simon,
> >
> > On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
> > >
> > > This chip is used on coral and we need to generate ACPI tables for sound
> > > to make it work. Add a driver that does just this (i.e. at present does
> > > not actually support playing sound).
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > >
> > > Changes in v2:
> > > Add a comment about only x86 boards supporting NHLT
> > >
> > > Changes in v1:
> > > - Use acpi,ddn instead of acpi,desc
> > > - Drop the unwanted acpi_device_write_gpio_desc()
> > > - Rename max97357a to max98357a
> > > - Add NHLT support
> > > - Capitalise ACPI_OPS_PTR
> > > - Rebase to master
> > >
> > >  configs/sandbox_defconfig|   1 +
> > >  doc/device-tree-bindings/sound/max98357a.txt |  22 +++
> > >  drivers/sound/Kconfig|   9 ++
> > >  drivers/sound/Makefile   |   1 +
> > >  drivers/sound/max98357a.c| 161 +++
> > >  5 files changed, 194 insertions(+)
> > >  create mode 100644 doc/device-tree-bindings/sound/max98357a.txt
> > >  create mode 100644 drivers/sound/max98357a.c
> > >
> > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > index 3817091d02..0c9524ff6c 100644
> > > --- a/configs/sandbox_defconfig
> > > +++ b/configs/sandbox_defconfig
> > > @@ -207,6 +207,7 @@ CONFIG_SMEM=y
> > >  CONFIG_SANDBOX_SMEM=y
> > >  CONFIG_SOUND=y
> > >  CONFIG_SOUND_DA7219=y
> > > +CONFIG_SOUND_MAX98357A=y
> > >  CONFIG_SOUND_SANDBOX=y
> > >  CONFIG_SANDBOX_SPI=y
> > >  CONFIG_SPMI=y
> > > diff --git a/doc/device-tree-bindings/sound/max98357a.txt 
> > > b/doc/device-tree-bindings/sound/max98357a.txt
> > > new file mode 100644
> > > index 00..4bce14ce80
> > > --- /dev/null
> > > +++ b/doc/device-tree-bindings/sound/max98357a.txt
> > > @@ -0,0 +1,22 @@
> > > +Maxim MAX98357A audio DAC
> > > +
> > > +This node models the Maxim MAX98357A DAC.
> > > +
> > > +Required properties:
> > > +- compatible   : "maxim,max98357a"
> > > +
> > > +Optional properties:
> > > +- sdmode-gpios : GPIO specifier for the chip's SD_MODE pin.
> > > +If this option is not specified then driver does not manage
> > > +the pin state (e.g. chip is always on).
> > > +- sdmode-delay : specify delay time for SD_MODE pin.
> > > +If this option is specified, which means it's required i2s clocks
> > > +ready before SD_MODE is unmuted in order to avoid the speaker 
> > > pop noise.
> > > +It's observed that 5ms is sufficient.
> > > +
> > > +Example:
> > > +
> > > +max98357a {
> > > +   compatible = "maxim,max98357a";
> > > +   sdmode-gpios = <&qcom_pinmux 25 0>;
> > > +};
> > > diff --git a/drivers/sound/Kconfig b/drivers/sound/Kconfig
> > > index 7f214b97be..0948d8caab 100644
> > > --- a/drivers/sound/Kconfig
> > > +++ b/drivers/sound/Kconfig
> > > @@ -113,6 +113,15 @@ config SOUND_MAX98095
> > >   audio data and I2C for codec control. At present it only works
> > >   with the Samsung I2S driver.
> > >
> > > +config SOUND_MAX98357A
> > > +   bool "Support Maxim max98357a audio codec"
> > > +   depends on PCI
> > > +   help
> > > + Enable the max98357a audio codec. This is connected on PCI for
> > > + audio data codec control. This is currently only capable of 
> > > providing
> > > + ACPI information. A full driver (with sound in U-Boot) is 
> > > currently
> > > + not available.
> > > +
> > >  config SOUND_RT5677
> > > bool "Support Realtek RT5677 audio codec"
> > > depends on SOUND
> > > diff --git a/drivers/sound/Makefile b/drivers/sound/Makefile
> > > index 8c3933ad15..9b40c8012f 100644
> > > --- a/drivers/sound/Makefile
> > > +++ b/drivers/sound/Makefile
> > > @@ -17,6 +17,7 @@ obj-$(CONFIG_SOUND_WM8994)+= wm8994.o
> > >  obj-$(CONFIG_SOUND_MAX98088)   += max98088.o maxim_codec.o
> > >  obj-$(CONFIG_SOUND_MAX98090)   += max98090.o maxim_codec.o
> > >  obj-$(CONFIG_SOUND_MAX98095)   += max98095.o maxim_codec.o
> > > +obj-$(CONFIG_SOUND_MAX98357A)  += max98357a.o
> > >  obj-$(CONFIG_SOUND_INTEL_HDA)  += hda_codec.o
> > >  obj-$(CONFIG_SOUND_I8254)  += i8254_beep.o
> > >  obj-$(CONFIG_SOUND_RT5677) += rt5677.o
> > > diff --git a/drivers/sound/max98357a.c b/drivers/sound/max98357a.c
> > > new file mode 100644
> > > index 00..827262d235
> > > --- /dev/null
> > > +++ b/drivers/sound/max98357a.c
> > > @@ -0,0 +1,161 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * max98357a.c -- MAX98357A Audio driver
> > > + *
> > > + * Copyright 2019 Google LLC
> > > + * Parts taken from coreboot
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#ifdef CONFIG_X86
> > > +#in

Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table

[Simon Glass]

Hi Bin,

On Mon, 13 Jul 2020 at 00:10, Bin Meng  wrote:
>
> Hi Simon,
>
> On Mon, Jul 13, 2020 at 3:37 AM Simon Glass  wrote:
> >
> > Hi Bin,
> >
> > On Tue, 7 Jul 2020 at 21:25, Simon Glass  wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 6 Jul 2020 at 18:22, Bin Meng  wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass  wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Thu, 2 Jul 2020 at 22:33, Bin Meng  wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass  
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng  wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Bin,
> > > > > > > > >
> > > > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng  
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) 
> > > > > > > > > > > table describes the
> > > > > > > > > > > audio codecs and connections in a system. Various devices 
> > > > > > > > > > > can contribute
> > > > > > > > > > > information to produce the table.
> > > > > > > > > > >
> > > > > > > > > > > Add core support for this, based on a structure which is 
> > > > > > > > > > > built up through
> > > > > > > > > > > calls to the driver.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Simon Glass 
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > >  drivers/core/acpi.c | 15 +++
> > > > > > > > > > >  include/dm/acpi.h   | 26 ++
> > > > > > > > > > >  2 files changed, 41 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > > > > +   METHOD_SETUP_NHLT,
> > > > > > > > > >
> > > > > > > > > > Do we really need to promote this to be an ACPI core 
> > > > > > > > > > method? Can we
> > > > > > > > > > reuse the SSDT/DSDT one?
> > > > > > > > >
> > > > > > > > > I don't think so. Those two are for a particular purpose. In 
> > > > > > > > > fact NHLT
> > > > > > > > > is generated while doing SSDT I think. The idea is that 
> > > > > > > > > drivers that
> > > > > > > > > want to contribute to NHLT can do so. But we cannot use the 
> > > > > > > > > SSDT
> > > > > > > > > mechanism since each driver contributes only a part of the 
> > > > > > > > > info, and
> > > > > > > > > we need something else to bring it all together.
> > > > > > > >
> > > > > > > > Will there be only one device that sets up the NHLT info?
> > > > > > >
> > > > > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > > > > suppose it depends on the audio codecs present.
> > > > > >
> > > > > > Could we make this method to be provided by the codec device, 
> > > > > > instead
> > > > > > of a generic ACPI core method?
> > > > >
> > > > > The codec device does implement this. See the drivers where they
> > > > > actually implement the NHLT method.
> > > > >
> > > > > This is definitely an ACPI-specific thing, so I think we need core
> > > > > support for iterating through drivers that want to provide this info.
> > > >
> > > > My concern is that this is not generic enough to promote this to ACPI 
> > > > core.
> > > >
> > > > I wanted to have something like this:
> > > >
> > > > Create a codec uclass driver, and in the code uclass driver, create an
> > > > op that is used to set up the NHLT infor if ACPI_GEN is on.
> > >
> > > We already have UCLASS_SOUND so could add it to sound_ops. But that
> > > seems weird to me since this is not an operation to play a sound. We
> > > do this with GIPOs and IRQs but in that case the operation returns
> > > some data. Here we are asking the driver to add some data to a table.
> > > I'm just not sure it makes sense.
> > >
> > > What do you think?
> >
> > Let me know what you think about this as I'd like to finish this series.
>
> I haven't looked into the details of NHLT but I trust your
> consideration so let's leave this as it is.

I think both approaches require bending in some direction due to the
fact that the data structure is 'built up' during the calls, but not
added to ACPI until all calls are done.

I'll try a little series to switch it the other way so we can compare it.

Regards,
Simon

Re: [PATCH 1/2] patman: Make sure sendemail.suppresscc is (un)set correctly

[Simon Glass]

On Sun, 12 Jul 2020 at 20:50, Nicolas Boichat  wrote:
>
> Setting sendemail.suppresscc to all or cccmd leads to --cc-cmd
> parameter being ignored, and emails going either nowhere, or
> just to the To: line maintainer.
>
> Signed-off-by: Nicolas Boichat 
> ---
>
>  tools/patman/gitutil.py | 25 +
>  tools/patman/main.py|  2 ++
>  2 files changed, 27 insertions(+)

Reviewed-by: Simon Glass 

Re: [PATCH 2/2] patman: When no tracking branch is provided, tell the user

[Simon Glass]

On Sun, 12 Jul 2020 at 20:50, Nicolas Boichat  wrote:
>
> The user can either count the number of patches, or provide a
> tracking branch.
>
> Signed-off-by: Nicolas Boichat 
> ---
>
>  tools/patman/main.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Rtp]

Simon South  writes:

> Peter Robinson  writes:
>> It should be fixed in the main devel repo, commit 3a5771249
>
> That commit enables the OHCI driver but doesn't lead to a working
> keyboard, at least for me.
>
> Have you actually tested this successfully on a PBP? (Has anyone else?)
> I wonder if there's something different about my unit.

The keyboard is working fine here. Maybe you could provide more details ?
Did you check if everything needed is enabled in your configuration and
if the keyboard is detected after a usb start / usb tree ?
Your keyboard is working with linux, right ?

Arnaud

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Simon South]

Arnaud Patard (Rtp)  writes:
> Did you check if everything needed is enabled in your configuration and
> if the keyboard is detected after a usb start / usb tree ?

It is detected, yes.

I believe the configuration is fine; I see this issue using the standard
"pinebook-pro-rk3399_defconfig" and building either from the latest
commit in git or from the tree at commit 3a5771249 (Peter's, which he
mentioned earlier).

Changing the configuration to remove the XHCI and EHCI drivers, or to
add the Inno USB phy driver, doesn't help.

Beyond that, for BL31 I'm using a release build of v2.3 of the ARM
Trusted Firmware. I've tested using debug and release builds of v2.3 and
v2.2 without seeing a difference.

> Your keyboard is working with linux, right ?

Yes it is. It's only U-Boot that's affected.

The only things I've thought of so far that might be unique about my
setup are

- My PBP is from the latest manufacturing run of a few months ago, not
  last year's; and

- I currently have the eMMC module disabled via the internal switch and
  am booting off a microSD card.

Otherwise I'm at a total loss. The driver sets everything up correctly
and then the controller just does nothing. In GNU/Linux, everything
works fine. Very strange.

-- 
Simon South
si...@simonsouth.net

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Peter Robinson]

> > Did you check if everything needed is enabled in your configuration and
> > if the keyboard is detected after a usb start / usb tree ?
>
> It is detected, yes.
>
> I believe the configuration is fine; I see this issue using the standard
> "pinebook-pro-rk3399_defconfig" and building either from the latest
> commit in git or from the tree at commit 3a5771249 (Peter's, which he
> mentioned earlier).
>
> Changing the configuration to remove the XHCI and EHCI drivers, or to
> add the Inno USB phy driver, doesn't help.
>
> Beyond that, for BL31 I'm using a release build of v2.3 of the ARM
> Trusted Firmware. I've tested using debug and release builds of v2.3 and
> v2.2 without seeing a difference.

I believe I have 2.3 from upstream.

> > Your keyboard is working with linux, right ?
>
> Yes it is. It's only U-Boot that's affected.
>
> The only things I've thought of so far that might be unique about my
> setup are
>
> - My PBP is from the latest manufacturing run of a few months ago, not
>   last year's; and
>
> - I currently have the eMMC module disabled via the internal switch and
>   am booting off a microSD card.

I'm booting off mSD and I actually took out the eMMC because it was a
pain dealing with it when I was developing the upstream U-Boot
support. I think my keyboard firmware is quite old (dealing with that
is on my list), but I'm not sure that should make a difference here.

Re: [GIT PULL] TI changes for v2020.10-rc1

[Tom Rini]

On Tue, Jul 14, 2020 at 11:08:44AM +0530, Lokesh Vutla wrote:

> Hi Tom,
>   Please find the pull request for v2020.10-rc1 containing TI specific 
> changes.
> 
> Travis-CI build: 
> https://travis-ci.org/github/lokeshvutla/u-boot/builds/707679753
> 
> The following changes since commit 497c7598c4e713eb9ad88fd7963e57b21b8b35e1:
> 
>   Merge branch 'master' of 
> https://gitlab.denx.de/u-boot/custodians/u-boot-spi (2020-07-11 17:40:00 
> -0400)
> 
> are available in the Git repository at:
> 
>   https://gitlab.denx.de/u-boot/custodians/u-boot-ti.git tags/ti-v2020.10-rc1
> 
> for you to fetch changes up to 865fdfddce467f446b64f2aa7ba77f9dd0a48a6b:
> 
>   arm: k3: use correct weak function name spl_board_prepare_for_linux 
> (2020-07-13 20:58:34 +0530)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] memsize: Make get_ram_size() work with arbitary RAM size

[Heinrich Schuchardt]

On 14.07.20 12:35, Bin Meng wrote:
> Hi Wolfgang,
>
> On Tue, Jul 14, 2020 at 1:18 AM Wolfgang Denk  wrote:
>>
>> Dear Bin Meng,
>>
>> In message <1594378641-26360-1-git-send-email-bmeng...@gmail.com> you wrote:
>>>
>>> Currently get_ram_size() only works with certain RAM size like 1GiB,
>>> 2GiB, 4GiB, 8GiB, etc. Chanage the codes to work with any RAM size.
>>
>> I'm afraid I don't understand this change,  Can you please explain a
>> bit more detailed what "any RAM size" means?
>
> I meant "any RAM size" that is not power of two.
>
>>
>> The existing code should work fine with any RAM size that is a power
>> of two (and the algoithm used is based on this assumption, so the
>> code would fail if it is not met).
>
> Unfortunately this limitation is not documented clearly.
>
>>
>>> - long   save[BITS_PER_LONG - 1];
>>> - long   save_base;
>>> - long   cnt;
>>> - long   val;
>>> - long   size;
>>> - inti = 0;
>>> + long save[BITS_PER_LONG - 1];
>>> + long save_base;
>>> + long cnt;
>>> + long val;
>>> + long size;
>>> + int i = 0;
>>
>> Why do you change the formatting here?  I can see no need for that?
>
> I see the above are the only places in this file that do not follow
> our coding practices. Since I was modifying this function, I think
> it's fine to do the updates while we are here.
>
>>
>>
>>> + long n = maxsize / sizeof(long);
>>>
>>> - for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) {
>>> + n = __ffs(n);
>>> + n = BIT(n);
>>> +
>>> + for (cnt = n >> 1; cnt > 0; cnt >>= 1) {
>>
>> I can only speculate - but do you think that this will work for a
>> memory size of - for example - 2.5 GiB as might result from combining
>> two banks of 2 GiB resp. 512 MiB ?   I bet it doesn't.
>
> Correct. This issue was exposed when I was testing U-Boot with QEMU on
> RISC-V. With QEMU we can instantiate arbitrary RAM size for a given
> machine.
>
>>
>> For correct operation (as originally intended) you would always
>> specify a maxsize twice as large as the maximum possible memory size
>> of a bank of memory, and the function would return the real size it
>> found.
>
> I don't think this function can work as expected. Even if I pass a
> power-of-two RAM size to this function, it could still fail. Imagine
> the target has 2GiB memory size, and I pass 8GiB as the maxsize to
> this function. The function will hang when it tries to read/write
> something at an address that is beyond 2GiB.
>
>>
>> Any other use, especially not checking one bank of memory at a time,
>> will not work as intended.  And I have yet to see systems where
>> the size of a bank of memory is not a power of two.
>>
>> So I feel what you are doing here is not an improvement, but a
>> "workaround" for some incorrect usage.
>>
>
> Given what you mentioned the function limitation, we should update the
> codes with the above power of two assumptions documented.
>
>> I don't think we should accept this patch.
>>
>
> Regards,
> Bin
>


If we want a fast algorithm to determine the last supported address even
if the start or size is not a power of two:

unsigned long n = sizeof(unsigned long) * 8 - 1;
unsigned long addr = 0;
n = 1UL << n;
for (; n; n >>= 1) {
unsigned long next = addr + n;

if (next < start || next <= start + size - 1 && test(next))
addr = next;
}

Best regards

Heinrich

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Rtp]

Simon South  writes:

> Arnaud Patard (Rtp)  writes:
>> Did you check if everything needed is enabled in your configuration and
>> if the keyboard is detected after a usb start / usb tree ?
>
> It is detected, yes.

ok.

>
> I believe the configuration is fine; I see this issue using the standard
> "pinebook-pro-rk3399_defconfig" and building either from the latest
> commit in git or from the tree at commit 3a5771249 (Peter's, which he
> mentioned earlier).

Did you check the stdin variable content if it contains only "usbkbd" ?
If you have more than one value in it (like "serial,usbkbd"), do you have
CONSOLE_MUX configuration setting enabled ? If it's not enabled, test
with it, as I guess you probably want stdin set to "serial,usbkbd".

>
> Changing the configuration to remove the XHCI and EHCI drivers, or to
> add the Inno USB phy driver, doesn't help.
>
> Beyond that, for BL31 I'm using a release build of v2.3 of the ARM
> Trusted Firmware. I've tested using debug and release builds of v2.3 and
> v2.2 without seeing a difference.
>

imho, ATF has nothing to do with your issue and it's possibly a uboot
configuration issue (like stdin variable or .config).

>> Your keyboard is working with linux, right ?
>
> Yes it is. It's only U-Boot that's affected.

So it's not hardware. I've tested fairly recent version of uboot (HEAD
pointing to 7012865e961ca2645d783adf4b75ca4abdbfe5a7) last week and the
keyboard was fine. That's why I'm really thinking of uboot
configuration.

>
> The only things I've thought of so far that might be unique about my
> setup are
>
> - My PBP is from the latest manufacturing run of a few months ago, not
>   last year's; and
>
> - I currently have the eMMC module disabled via the internal switch and
>   am booting off a microSD card.

Same here.

Arnaud

Re: [PATCH] memsize: Make get_ram_size() work with arbitary RAM size

[Wolfgang Denk]

Dear Heinrich,

In message <53dad1c7-7684-f975-1567-6ec5e03fa...@gmx.de> you wrote:
>
> If we want a fast algorithm to determine the last supported address even
> if the start or size is not a power of two:

Are you sure?  How is this supposed to work?

Running it with start = 0 and size = 0xC000 it will test the
memory locations

0x8000
0xA000
0xB000
0xB800
0xBC00
0xBE00
0xBF00
0xBF80
0xBFC0
0xBFE0
0xBFF0
0xBFF8
0xBFFC
0xBFFE
0xBFFF
0xBFFF8000
0xBFFFC000
0xBFFFE000
0xB000
0xB800
0xBC00
0xBE00
0xBF00
0xBF80
0xBFC0
0xBFE0
0xBFF0
0xBFF8
0xBFFC
0xBFFE
0xBFFF

What do you intend with such a sequence?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
The trouble with our times is that the future is not what it used  to
be. - Paul Valery

[PATCH v2 1/1] efi_loader: update secure state

[Heinrich Schuchardt]

Update the UEFI secure state when variable 'PK' is updated in the TEE
variables implementation.

Signed-off-by: Heinrich Schuchardt 
---
v2:
simplify the logic for handling return codes
---
 lib/efi_loader/efi_variable_tee.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/efi_loader/efi_variable_tee.c 
b/lib/efi_loader/efi_variable_tee.c
index 24e0663ebd..ddefa93f8f 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -557,6 +557,12 @@ efi_status_t efi_set_variable_int(u16 *variable_name, 
const efi_guid_t *vendor,
var_property.maxsize = var_acc->data_size;
ret = set_property_int(variable_name, name_size, vendor, 
&var_property);
}
+
+   if (alt_ret != EFI_SUCCESS)
+   goto out;
+
+   if (!u16_strcmp(variable_name, L"PK"))
+   alt_ret = efi_init_secure_state();
 out:
free(comm_buf);
return alt_ret == EFI_SUCCESS ? ret : alt_ret;
@@ -716,5 +722,9 @@ efi_status_t efi_init_variables(void)
  MM_VARIABLE_COMMUNICATE_SIZE +
  max_payload_size;

+   ret = efi_init_secure_state();
+   if (ret != EFI_SUCCESS)
+   return ret;
+
return EFI_SUCCESS;
 }
--
2.27.0

Re: Pinebook Pro keyboard (RK3399 OHCI)?

[Simon South]

Arnaud Patard (Rtp)  writes:
> Did you check the stdin variable content if it contains only "usbkbd" ?
> If you have more than one value in it (like "serial,usbkbd"), do you have
> CONSOLE_MUX configuration setting enabled ?

Yes, "stdin" is set to "serial,usbkbd" and CONSOLE_MUX is set. The
console works fine via the serial connection, apart from being sluggish
while USB is enabled.

> Same here.

I just now tried removing another variable by building U-Boot from a
fresh checkout on the PBP itself, rather than cross-compiling from my
x86_64 machine as I normally do. Same result.

So I'm completely at a loss.

If you have them handy, would you be willing to email me (off-list) your
idbloader.img and u-boot.itb files please so I can try booting them on
my hardware? I'm starting to think at this point I somehow got a lemon,
but if your build works that would at least point to something I've
missed in my setup.

-- 
Simon South
si...@simonsouth.net

[PATCH 1/1] efi_loader: configuration of variables store

[Heinrich Schuchardt]

The file based and the OP-TEE based UEFI variable store are mutually
exclusive. Define them as choice options in Kconfig.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/Kconfig | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 4324694d48..8827c76cc9 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -27,13 +27,28 @@ config EFI_LOADER

 if EFI_LOADER

+choice
+   prompt "Store for non-volatile UEFI variables"
+   default EFI_VARIABLE_FILE_STORE
+   help
+ Select where non-volatile UEFI variables shall be stored.
+
 config EFI_VARIABLE_FILE_STORE
bool "Store non-volatile UEFI variables as file"
depends on FAT_WRITE
-   default y
help
- Select tis option if you want non-volatile UEFI variables to be stored
- as file /ubootefi.var on the EFI system partition.
+ Select this option if you want non-volatile UEFI variables to be
+ stored as file /ubootefi.var on the EFI system partition.
+
+config EFI_MM_COMM_TEE
+   bool "UEFI variables storage service via OP-TEE"
+   depends on OPTEE
+   help
+ If OP-TEE is present and running StandAloneMM, dispatch all UEFI
+ variable related operations to that. The application will verify,
+ authenticate and store the variables on an RPMB.
+
+endchoice

 config EFI_GET_TIME
bool "GetTime() runtime service"
@@ -174,13 +189,4 @@ config EFI_SECURE_BOOT
  it is signed with a trusted key. To do that, you need to install,
  at least, PK, KEK and db.

-config EFI_MM_COMM_TEE
-   bool "UEFI variables storage service via OP-TEE"
-   depends on OPTEE
-   default n
-   help
- If OP-TEE is present and running StandAloneMM, dispatch all UEFI 
variable
- related operations to that. The application will verify, authenticate 
and
- store the variables on an RPMB.
-
 endif
--
2.27.0

Re: Using gerrit or github for review?

[Karsten Merker]

On Mon, Jul 13, 2020 at 06:05:42PM -0400, Corey Clayton wrote:
> On Mon, Jul 13, 2020 at 12:25:42PM -0600, Simon Glass wrote:
> 
> > At present U-Boot uses the mailing list for patch review. What do
> > people think about trying out geritt or github for this? I'd be
> > willing to do a trial with the -dm mailing list.
> 
> This is both my first message to the mailing list and my first
> email sent using mutt.  I'm hoping to eventually participate
> with patches and reviews but the mailing-list-driven
> developement model has been a barrier for myself an probably
> many others.  I'm slowly trying to climb over it now but some
> will never find the time.  Perhaps a good question is: How long
> does it take to learn the mailing-list workflow vs the github
> workflow?
> 
> If u-boot was using github, I would have contributed long ago
> and I suspect there are others in the same bucket.  Thats my
> perspective at least :)

Hello,

to provide a different perspective: if U-Boot would have done
everything inside github instead of using its traditional
mailinglist-based workflow, I would never have contributed to
U-Boot, and moving everything from the mailinglist to github
would make any future contributions infeasible for me.

The github workflow makes it impossible to open an issue or to
comment on an existing issue or to provide feedback about a patch
without being a github customer, and becoming a github customer
is not an option for me (and quite a number of other open-source
developers) as the github TOS contain clauses that I (and other
people) consider completely unacceptable.

Besides the aforementioned points I am generally concerned about
the closed nature of the github issue- and pull-request system. 
While it is of course easily possible to move a git repo from
github to somewhere else, it is as far as I know (please correct
me if I should me misinformed here) not possible to export the
comments and discussions in issues and pull requests in any
meaningful way to some other hosting platform, which creates a
strong vendor-lock-in once a project starts using the github
issue and pull-request facilities.  With the traditional
mailinglist-based workflow on the other hand, moving everything
to another hosting platform is trivial, so vendor-lock-in
is not a problem there.

Another problem that I see in the github workflow is that it
requires being online all the time while the mailinglist-based
workflow makes it easy to read and comment on patches while being
offline.  I am sure that many people will now think "everybody is
online all day nowadays", but that's not true everywhere.  I for
example travel a lot by train and use the time on the train for
catching up with current developments and for reviewing things. 
Where I live, for most practical purposes being on the train
effectively means being offline as far as modern web applications
are concerned.  Availability of mobile internet access is spotty
at best, and if one has internet connectivity inside the train at
all, it is often so slow that using it for interactive work on a
web interface is not feasible.  Receiving, writing and sending
emails on the other hand works without problems even with spotty
and slow internet connectivity.

Regards,
Karsten
-- 
Hiermit widerspreche ich ausdrücklich der Nutzung sowie der Weitergabe
meiner personenbezogenen Daten für Zwecke der Werbung sowie der Markt-
oder Meinungsforschung.

Re: [PATCH v2 1/1] efi_loader: update secure state

[ilias . apalodimas]

On Tue, Jul 14, 2020 at 06:08:15PM +0200, Heinrich Schuchardt wrote:
> Update the UEFI secure state when variable 'PK' is updated in the TEE
> variables implementation.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
>   simplify the logic for handling return codes
> ---
>  lib/efi_loader/efi_variable_tee.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_variable_tee.c 
> b/lib/efi_loader/efi_variable_tee.c
> index 24e0663ebd..ddefa93f8f 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -557,6 +557,12 @@ efi_status_t efi_set_variable_int(u16 *variable_name, 
> const efi_guid_t *vendor,
>   var_property.maxsize = var_acc->data_size;
>   ret = set_property_int(variable_name, name_size, vendor, 
> &var_property);
>   }
> +
> + if (alt_ret != EFI_SUCCESS)
> + goto out;
> +
> + if (!u16_strcmp(variable_name, L"PK"))
> + alt_ret = efi_init_secure_state();
>  out:
>   free(comm_buf);
>   return alt_ret == EFI_SUCCESS ? ret : alt_ret;
> @@ -716,5 +722,9 @@ efi_status_t efi_init_variables(void)
> MM_VARIABLE_COMMUNICATE_SIZE +
> max_payload_size;
> 
> + ret = efi_init_secure_state();
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
>   return EFI_SUCCESS;
>  }
> --
> 2.27.0
> 

Just a note here this depends on [1].

With that applied
Reviewed-by: Ilias Apalodimas 

[1] https://lists.denx.de/pipermail/u-boot/2020-July/419473.html

Re: [PATCH v2 1/1] Dockerfile: provide kernel for libguestfs-tools

[Tom Rini]

On Tue, Jul 14, 2020 at 08:18:56AM +0200, Heinrich Schuchardt wrote:

> The libguestfs-tools use QEMU to mount an image file. This requires a Linux
> kernel.
> 
> On Ubuntu the kernel (/boot/vmlinuz*) is not readable for normal users
> (chmod 600), cf.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/759725
> 
> Install a kernel and make it readable for all users (chmod 644).
> 
> Signed-off-by: Heinrich Schuchardt 

This causes the tests to fail now that they're trying to use
libguestfs-tools:
https://gitlab.denx.de/u-boot/u-boot/-/jobs/124872

I did a quick change to pass in the KVM group to useradd as well, but
that didn't catch.  I suspect that changing /dev/kvm inside the
container won't stick either.  But that shouldn't be fatal as it's still
fast enough.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 1/1] Dockerfile: provide kernel for libguestfs-tools

[Heinrich Schuchardt]

Am 14. Juli 2020 23:28:21 MESZ schrieb Tom Rini :
>On Tue, Jul 14, 2020 at 08:18:56AM +0200, Heinrich Schuchardt wrote:
>
>> The libguestfs-tools use QEMU to mount an image file. This requires a
>Linux
>> kernel.
>> 
>> On Ubuntu the kernel (/boot/vmlinuz*) is not readable for normal
>users
>> (chmod 600), cf.
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/759725
>> 
>> Install a kernel and make it readable for all users (chmod 644).
>> 
>> Signed-off-by: Heinrich Schuchardt 
>
>This causes the tests to fail now that they're trying to use
>libguestfs-tools:
>https://gitlab.denx.de/u-boot/u-boot/-/jobs/124872
>
>I did a quick change to pass in the KVM group to useradd as well, but
>that didn't catch.  I suspect that changing /dev/kvm inside the
>container won't stick either.  But that shouldn't be fatal as it's
>still
>fast enough.

KVM requires docker --privileged according to what I read.

Tests failing that were not excercised before seems to be a step into the right 
direction. - But a lot of work before us.

Thanks for the update.

Best regards

Heinrich

Re: [PATCH v2 1/1] Dockerfile: provide kernel for libguestfs-tools

[Tom Rini]

On Wed, Jul 15, 2020 at 12:00:25AM +0200, Heinrich Schuchardt wrote:
> Am 14. Juli 2020 23:28:21 MESZ schrieb Tom Rini :
> >On Tue, Jul 14, 2020 at 08:18:56AM +0200, Heinrich Schuchardt wrote:
> >
> >> The libguestfs-tools use QEMU to mount an image file. This requires a
> >Linux
> >> kernel.
> >> 
> >> On Ubuntu the kernel (/boot/vmlinuz*) is not readable for normal
> >users
> >> (chmod 600), cf.
> >> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/759725
> >> 
> >> Install a kernel and make it readable for all users (chmod 644).
> >> 
> >> Signed-off-by: Heinrich Schuchardt 
> >
> >This causes the tests to fail now that they're trying to use
> >libguestfs-tools:
> >https://gitlab.denx.de/u-boot/u-boot/-/jobs/124872
> >
> >I did a quick change to pass in the KVM group to useradd as well, but
> >that didn't catch.  I suspect that changing /dev/kvm inside the
> >container won't stick either.  But that shouldn't be fatal as it's
> >still
> >fast enough.
> 
> KVM requires docker --privileged according to what I read.
> 
> Tests failing that were not excercised before seems to be a step into the 
> right direction. - But a lot of work before us.

It's not progress as they do pass when I apply the patch I posted the
other day to fix sudo'ing the tests.  And we may need to have an
off-list chat to make sure everyone with a runner is configured
consistently.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 1/1] efi_loader: configuration of variables store

[ilias . apalodimas]

On Tue, Jul 14, 2020 at 07:28:43PM +0200, Heinrich Schuchardt wrote:
> The file based and the OP-TEE based UEFI variable store are mutually
> exclusive. Define them as choice options in Kconfig.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/efi_loader/Kconfig | 30 ++
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 4324694d48..8827c76cc9 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -27,13 +27,28 @@ config EFI_LOADER
> 
>  if EFI_LOADER
> 
> +choice
> + prompt "Store for non-volatile UEFI variables"
> + default EFI_VARIABLE_FILE_STORE
> + help
> +   Select where non-volatile UEFI variables shall be stored.
> +
>  config EFI_VARIABLE_FILE_STORE
>   bool "Store non-volatile UEFI variables as file"
>   depends on FAT_WRITE
> - default y
>   help
> -   Select tis option if you want non-volatile UEFI variables to be stored
> -   as file /ubootefi.var on the EFI system partition.
> +   Select this option if you want non-volatile UEFI variables to be
> +   stored as file /ubootefi.var on the EFI system partition.
> +
> +config EFI_MM_COMM_TEE
> + bool "UEFI variables storage service via OP-TEE"
> + depends on OPTEE
> + help
> +   If OP-TEE is present and running StandAloneMM, dispatch all UEFI
> +   variable related operations to that. The application will verify,
> +   authenticate and store the variables on an RPMB.
> +
> +endchoice
> 
>  config EFI_GET_TIME
>   bool "GetTime() runtime service"
> @@ -174,13 +189,4 @@ config EFI_SECURE_BOOT
> it is signed with a trusted key. To do that, you need to install,
> at least, PK, KEK and db.
> 
> -config EFI_MM_COMM_TEE
> - bool "UEFI variables storage service via OP-TEE"
> - depends on OPTEE
> - default n
> - help
> -   If OP-TEE is present and running StandAloneMM, dispatch all UEFI 
> variable
> -   related operations to that. The application will verify, authenticate 
> and
> -   store the variables on an RPMB.
> -
>  endif
> --
> 2.27.0
> 

Acked-by: Ilias Apalodimas 

Re: [PATCH v4 06/15] board: ns3: default reset type to L3

[Simon Glass]

On Fri, 10 Jul 2020 at 02:53, Rayagonda Kokatanur
 wrote:
>
> Default "reset" from U-Boot to L3 reset.
> "reset" command with argument will trigger L1 reset.
>
> Signed-off-by: Rajesh Ravi 
> Signed-off-by: Bharat Kumar Reddy Gooty 
> Signed-off-by: Rayagonda Kokatanur 
> ---
> Changes from v3:
>   -Address review comments from Simon,
>Update commit message ie change u-boot to U-Boot
>
>  board/broadcom/bcmns3/ns3.c | 20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

See below

> diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> index 5e644bd466..1221f26ddc 100644
> --- a/board/broadcom/bcmns3/ns3.c
> +++ b/board/broadcom/bcmns3/ns3.c
> @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> return 0;
>  }
>
> -void reset_cpu(ulong addr)
> +void reset_cpu(ulong level)
>  {
> -   psci_system_reset();
> +#define L3_RESET 30

Put this at top of file and add a comment about what the value means.

> +   u32 reset_level, strap_val;
> +
> +   /* Default reset type is L3 reset */
> +   if (!level) {
> +   /*
> +* Encoding: u-boot reset command expects decimal argument

U-Boot


> +* strap val = 1st decimal digit;reset level = 2nd decimal 
> digit
> +*/
> +   strap_val = L3_RESET % 10;
> +   level = L3_RESET / 10;
> +   reset_level = level % 10;
> +   psci_system_reset2(reset_level, strap_val);
> +   } else {
> +   /* U-boot cmd "reset" with any arg will trigger L1 reset */
> +   psci_system_reset();
> +   }
>  }
> --
> 2.17.1
>

Re: [PATCH v2] serial: Fix SIFIVE debug serial dependency

[Simon Glass]

On Fri, 10 Jul 2020 at 04:41, Michal Simek  wrote:
>
> The commit 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for
> debug consoles") has added incorrect dependency for SIFIVE debug uart which
> should depend on SIFIVE driver instead of PL01x.
>
> Fixes: 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for debug 
> consoles")
> Signed-off-by: Michal Simek 
> ---
>
> Changes in v2:
> - Add fixes tag - asked by Simon
>
>  drivers/serial/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 

Re: [PATCH v2 01/14] Kconfig: Introduce CONFIG_SYS_HAS_SRAM

[Simon Glass]

On Fri, 10 Jul 2020 at 04:22, Ovidiu Panait  wrote:
>
> In order to be able to replace "#ifdef CONFIG_SYS_SRAM_BASE" sequences
> with the IS_ENABLED() equivalent, introduce a new boolean Kconfig option
> that signals whether the platform has SRAM support.
>
> Signed-off-by: Ovidiu Panait 
> ---
>
>  Kconfig | 11 +++
>  1 file changed, 11 insertions(+)

Reviewed-by: Simon Glass 

Re: [PATCH v2 04/14] Kconfig: Remove CONFIG_SYS_SRAM_START

[Simon Glass]

On Fri, 10 Jul 2020 at 04:23, Ovidiu Panait  wrote:
>
> Remove ad-hoc CONFIG_SYS_SRAM_START and use CONFIG_SYS_SRAM_BASE instead.
>
> Signed-off-by: Ovidiu Panait 
> ---
>
>  Kconfig  | 2 ++
>  include/configs/devkit8000.h | 3 ---
>  include/configs/tricorder.h  | 3 ---
>  scripts/config_whitelist.txt | 1 -
>  4 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH v2 03/14] Kconfig: Convert CONFIG_SYS_SRAM_SIZE to Kconfig

[Simon Glass]

On Fri, 10 Jul 2020 at 04:23, Ovidiu Panait  wrote:
>
> This converts ad-hoc CONFIG_SYS_SRAM_SIZE to Kconfig.
>
> Signed-off-by: Ovidiu Panait 
> ---
>
>  Kconfig   | 7 +++
>  include/configs/devkit8000.h  | 1 -
>  include/configs/pic32mzdask.h | 2 --
>  include/configs/tricorder.h   | 1 -
>  scripts/config_whitelist.txt  | 1 -
>  5 files changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH 2/3] Revert "lib: fdt: Split fdtdec_setup_memory_banksize()"

[Simon Glass]

On Fri, 10 Jul 2020 at 05:16, Michal Simek  wrote:
>
> This reverts commit 118f4d4559a4386fa87a1e2509e84a1986b24a34.
>
> There is no user of this split function that's why remove it.
>
> Signed-off-by: Michal Simek 
> ---
>
>  include/fdtdec.h | 19 ---
>  lib/fdtdec.c | 18 ++
>  2 files changed, 6 insertions(+), 31 deletions(-)
>

Reviewed-by: Simon Glass 

Re: [PATCH 3/3] Revert "lib: fdt: Split fdtdec_setup_mem_size_base()"

[Simon Glass]

On Fri, 10 Jul 2020 at 05:17, Michal Simek  wrote:
>
> This reverts commit 3ebe09d09a407f93022d945a205c5318239eb3f6.
>
> There is no user of this split function that's why remove it.
>
> Signed-off-by: Michal Simek 
> ---
>
>  include/fdtdec.h | 20 
>  lib/fdtdec.c | 11 +++
>  2 files changed, 3 insertions(+), 28 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH v2 2/3] lib: fdt: Convert fdtdes_setup_mem..() to livetree API

[Simon Glass]

On Fri, 10 Jul 2020 at 06:41, Michal Simek  wrote:
>
> Convert fdtdec_setup_mem_size_base(), get_next_memory_node(),
> fdtdec_setup_memory_banksize() and fdtdec_setup_mem_size_base_lowest() to
> livetree API.
>
> Tested on ZynqMP zcu104 board.
>
> Signed-off-by: Michal Simek 
> ---
>
> Changes in v2:
> - new patch asked by Simon
>
>  lib/fdtdec.c | 67 +++-
>  1 file changed, 35 insertions(+), 32 deletions(-)
>

Reviewed-by: Simon Glass 

Re: [PATCH 3/7] cmd: button: add a new 'button' command

[Simon Glass]

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Adds a command 'button' that provides the list of buttons
> supported by the board, and the state of a button.
>
> Signed-off-by: Philippe Reynes 
> ---
>  cmd/Kconfig  | 11 
>  cmd/Makefile |  1 +
>  cmd/button.c | 86 
> 
>  3 files changed, 98 insertions(+)
>  create mode 100644 cmd/button.c

Reviewed-by: Simon Glass 

Re: [PATCH v4 07/15] board: ns3: program GIC LPI tables

[Simon Glass]

On Fri, 10 Jul 2020 at 02:53, Rayagonda Kokatanur
 wrote:
>
> U-boot programs the GIC LPI configuration tables and enables
> the LPI table.
>
> Signed-off-by: Bharat Kumar Reddy Gooty 
> Signed-off-by: Rayagonda Kokatanur 
> ---
> Changes from v3:
>   -Address review comments from Simon,
>Use dt and a UCLASS_IRQ to get gic details instead of passing
>as arguments to function gic_lpi_tables_init().
>
>  board/broadcom/bcmns3/ns3.c | 10 ++
>  1 file changed, 10 insertions(+)

Reviewed-by: Simon Glass 

Re: Using gerrit or github for review?

[Simon Glass]

Hi,

On Tue, 14 Jul 2020 at 14:06, Karsten Merker  wrote:
>
> On Mon, Jul 13, 2020 at 06:05:42PM -0400, Corey Clayton wrote:
> > On Mon, Jul 13, 2020 at 12:25:42PM -0600, Simon Glass wrote:
> >
> > > At present U-Boot uses the mailing list for patch review. What do
> > > people think about trying out geritt or github for this? I'd be
> > > willing to do a trial with the -dm mailing list.
> >
> > This is both my first message to the mailing list and my first
> > email sent using mutt.  I'm hoping to eventually participate
> > with patches and reviews but the mailing-list-driven
> > developement model has been a barrier for myself an probably
> > many others.  I'm slowly trying to climb over it now but some
> > will never find the time.  Perhaps a good question is: How long
> > does it take to learn the mailing-list workflow vs the github
> > workflow?
> >
> > If u-boot was using github, I would have contributed long ago
> > and I suspect there are others in the same bucket.  Thats my
> > perspective at least :)
>
> Hello,
>
> to provide a different perspective: if U-Boot would have done
> everything inside github instead of using its traditional
> mailinglist-based workflow, I would never have contributed to
> U-Boot, and moving everything from the mailinglist to github
> would make any future contributions infeasible for me.
>
> The github workflow makes it impossible to open an issue or to
> comment on an existing issue or to provide feedback about a patch
> without being a github customer, and becoming a github customer
> is not an option for me (and quite a number of other open-source
> developers) as the github TOS contain clauses that I (and other
> people) consider completely unacceptable.
>
> Besides the aforementioned points I am generally concerned about
> the closed nature of the github issue- and pull-request system.
> While it is of course easily possible to move a git repo from
> github to somewhere else, it is as far as I know (please correct
> me if I should me misinformed here) not possible to export the
> comments and discussions in issues and pull requests in any
> meaningful way to some other hosting platform, which creates a
> strong vendor-lock-in once a project starts using the github
> issue and pull-request facilities.  With the traditional
> mailinglist-based workflow on the other hand, moving everything
> to another hosting platform is trivial, so vendor-lock-in
> is not a problem there.
>
> Another problem that I see in the github workflow is that it
> requires being online all the time while the mailinglist-based
> workflow makes it easy to read and comment on patches while being
> offline.  I am sure that many people will now think "everybody is
> online all day nowadays", but that's not true everywhere.  I for
> example travel a lot by train and use the time on the train for
> catching up with current developments and for reviewing things.
> Where I live, for most practical purposes being on the train
> effectively means being offline as far as modern web applications
> are concerned.  Availability of mobile internet access is spotty
> at best, and if one has internet connectivity inside the train at
> all, it is often so slow that using it for interactive work on a
> web interface is not feasible.  Receiving, writing and sending
> emails on the other hand works without problems even with spotty
> and slow internet connectivity.

Just to clarify, my question was whether we should add a new workflow.
I don't think there is any interest in removing the mailing-list flow.

Re your comments about the TOS - what specifically causes problems? Re
exporting comment I have been wondering that...is there no API for it?
Finally, do your comments apply to gerrit and gitlab as well?

Regards,
Simon

Re: [PATCH v4 02/15] arm: cpu: armv8: add L3 memory flush support

[Simon Glass]

On Fri, 10 Jul 2020 at 02:52, Rayagonda Kokatanur
 wrote:
>
> Add L3 memory flush support for NS3.
>
> Signed-off-by: Rayagonda Kokatanur 
> ---
> Changes from v3:
>   -Address review comments from Simon,
>Commit message correction
>
>  arch/arm/cpu/armv8/Makefile  |  1 +
>  arch/arm/cpu/armv8/bcmns3/Makefile   |  5 ++
>  arch/arm/cpu/armv8/bcmns3/lowlevel.S | 89 
>  3 files changed, 95 insertions(+)
>  create mode 100644 arch/arm/cpu/armv8/bcmns3/Makefile
>  create mode 100644 arch/arm/cpu/armv8/bcmns3/lowlevel.S

Reviewed-by: Simon Glass 

Can this be written in C?

Re: [PATCH 6/7] sandbox: enable button

[Simon Glass]

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Enable the support of button (driver and command) on sandbox.
>
> Signed-off-by: Philippe Reynes 
> ---
>  configs/sandbox_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass 

Re: [PATCH v4 14/15] doc: add README doc for bcmns3 platform

[Simon Glass]

On Fri, 10 Jul 2020 at 02:53, Rayagonda Kokatanur
 wrote:
>
> Add README doc for bcmns3 platform.
>
> Signed-off-by: Rayagonda Kokatanur 
> ---
>  doc/README.bcmns3 | 74 +++
>  1 file changed, 74 insertions(+)
>  create mode 100644 doc/README.bcmns3

Reviewed-by: Simon Glass 

Re: [PATCH v2 02/14] Kconfig: Convert CONFIG_SYS_SRAM_BASE to Kconfig

[Simon Glass]

On Fri, 10 Jul 2020 at 04:23, Ovidiu Panait  wrote:
>
> This converts ad-hoc CONFIG_SYS_SRAM_BASE to Kconfig.
>
> Signed-off-by: Ovidiu Panait 
> ---
>
>  Kconfig   | 5 +
>  include/configs/pic32mzdask.h | 1 -
>  scripts/config_whitelist.txt  | 1 -
>  3 files changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH] serial: Fix SIFIVE debug serial dependency

[Simon Glass]

On Fri, 10 Jul 2020 at 00:21, Michal Simek  wrote:
>
> Hi Simon,
>
> On 10. 07. 20 2:35, Simon Glass wrote:
> > Hi Michal,
> >
> > On Thu, 9 Jul 2020 at 08:17, Michal Simek  wrote:
> >>
> >> The commit 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for
> >> debug consoles") has added incorrect dependency for SIFIVE debug uart which
> >> should depend on SIFIVE driver instead of PL01x.
> >
> > Does that mean this should have a Fixes: tag?
>
> TBH I had it there but removed it because commit is pointing to it
> anyway. And also I am not aware about any stable process in place.
> Or is there any process around that people are looking for fixed tags
> and picking them to any u-boot stable tree?
>

+Tom Rini who may know

Re: [PATCH v2 17/44] sound: Add an ACPI driver for Maxim MAX98357ac

[Bin Meng]

Hi Simon,

On Tue, Jul 14, 2020 at 9:25 PM Simon Glass  wrote:
>
> Hi Bin,
>
> On Mon, 13 Jul 2020 at 23:01, Bin Meng  wrote:
> >
> > Hi Simon,
> >
> > On Tue, Jul 14, 2020 at 9:28 AM Bin Meng  wrote:
> > >
> > > Hi Simon,
> > >
> > > On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
> > > >
> > > > This chip is used on coral and we need to generate ACPI tables for sound
> > > > to make it work. Add a driver that does just this (i.e. at present does
> > > > not actually support playing sound).
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > > Changes in v2:
> > > > Add a comment about only x86 boards supporting NHLT
> > > >
> > > > Changes in v1:
> > > > - Use acpi,ddn instead of acpi,desc
> > > > - Drop the unwanted acpi_device_write_gpio_desc()
> > > > - Rename max97357a to max98357a
> > > > - Add NHLT support
> > > > - Capitalise ACPI_OPS_PTR
> > > > - Rebase to master
> > > >
> > > >  configs/sandbox_defconfig|   1 +
> > > >  doc/device-tree-bindings/sound/max98357a.txt |  22 +++
> > > >  drivers/sound/Kconfig|   9 ++
> > > >  drivers/sound/Makefile   |   1 +
> > > >  drivers/sound/max98357a.c| 161 +++
> > > >  5 files changed, 194 insertions(+)
> > > >  create mode 100644 doc/device-tree-bindings/sound/max98357a.txt
> > > >  create mode 100644 drivers/sound/max98357a.c
> > > >
> > > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > > index 3817091d02..0c9524ff6c 100644
> > > > --- a/configs/sandbox_defconfig
> > > > +++ b/configs/sandbox_defconfig
> > > > @@ -207,6 +207,7 @@ CONFIG_SMEM=y
> > > >  CONFIG_SANDBOX_SMEM=y
> > > >  CONFIG_SOUND=y
> > > >  CONFIG_SOUND_DA7219=y
> > > > +CONFIG_SOUND_MAX98357A=y
> > > >  CONFIG_SOUND_SANDBOX=y
> > > >  CONFIG_SANDBOX_SPI=y
> > > >  CONFIG_SPMI=y
> > > > diff --git a/doc/device-tree-bindings/sound/max98357a.txt 
> > > > b/doc/device-tree-bindings/sound/max98357a.txt
> > > > new file mode 100644
> > > > index 00..4bce14ce80
> > > > --- /dev/null
> > > > +++ b/doc/device-tree-bindings/sound/max98357a.txt
> > > > @@ -0,0 +1,22 @@
> > > > +Maxim MAX98357A audio DAC
> > > > +
> > > > +This node models the Maxim MAX98357A DAC.
> > > > +
> > > > +Required properties:
> > > > +- compatible   : "maxim,max98357a"
> > > > +
> > > > +Optional properties:
> > > > +- sdmode-gpios : GPIO specifier for the chip's SD_MODE pin.
> > > > +If this option is not specified then driver does not manage
> > > > +the pin state (e.g. chip is always on).
> > > > +- sdmode-delay : specify delay time for SD_MODE pin.
> > > > +If this option is specified, which means it's required i2s 
> > > > clocks
> > > > +ready before SD_MODE is unmuted in order to avoid the speaker 
> > > > pop noise.
> > > > +It's observed that 5ms is sufficient.
> > > > +
> > > > +Example:
> > > > +
> > > > +max98357a {
> > > > +   compatible = "maxim,max98357a";
> > > > +   sdmode-gpios = <&qcom_pinmux 25 0>;
> > > > +};
> > > > diff --git a/drivers/sound/Kconfig b/drivers/sound/Kconfig
> > > > index 7f214b97be..0948d8caab 100644
> > > > --- a/drivers/sound/Kconfig
> > > > +++ b/drivers/sound/Kconfig
> > > > @@ -113,6 +113,15 @@ config SOUND_MAX98095
> > > >   audio data and I2C for codec control. At present it only works
> > > >   with the Samsung I2S driver.
> > > >
> > > > +config SOUND_MAX98357A
> > > > +   bool "Support Maxim max98357a audio codec"
> > > > +   depends on PCI
> > > > +   help
> > > > + Enable the max98357a audio codec. This is connected on PCI for
> > > > + audio data codec control. This is currently only capable of 
> > > > providing
> > > > + ACPI information. A full driver (with sound in U-Boot) is 
> > > > currently
> > > > + not available.
> > > > +
> > > >  config SOUND_RT5677
> > > > bool "Support Realtek RT5677 audio codec"
> > > > depends on SOUND
> > > > diff --git a/drivers/sound/Makefile b/drivers/sound/Makefile
> > > > index 8c3933ad15..9b40c8012f 100644
> > > > --- a/drivers/sound/Makefile
> > > > +++ b/drivers/sound/Makefile
> > > > @@ -17,6 +17,7 @@ obj-$(CONFIG_SOUND_WM8994)+= wm8994.o
> > > >  obj-$(CONFIG_SOUND_MAX98088)   += max98088.o maxim_codec.o
> > > >  obj-$(CONFIG_SOUND_MAX98090)   += max98090.o maxim_codec.o
> > > >  obj-$(CONFIG_SOUND_MAX98095)   += max98095.o maxim_codec.o
> > > > +obj-$(CONFIG_SOUND_MAX98357A)  += max98357a.o
> > > >  obj-$(CONFIG_SOUND_INTEL_HDA)  += hda_codec.o
> > > >  obj-$(CONFIG_SOUND_I8254)  += i8254_beep.o
> > > >  obj-$(CONFIG_SOUND_RT5677) += rt5677.o
> > > > diff --git a/drivers/sound/max98357a.c b/drivers/sound/max98357a.c
> > > > new file mode 100644
> > > > index 00..827262d235
> > > > --- /dev/null
> > > > +++ b/drivers/sound/max98357a.c
> > > > @@ -0,0 +1,161 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * max983

Re: [PATCH v3 2/3] board: ns3: add FIT image its file

[Simon Glass]

On Fri, 10 Jul 2020 at 04:18, Rayagonda Kokatanur
 wrote:
>
> From: Pramod Kumar 
>
> Add FIT image its file.
>
> Signed-off-by: Pramod Kumar 
> Signed-off-by: Rayagonda Kokatanur 
> ---
>  board/broadcom/bcmns3/fit/multi.its | 59 +
>  1 file changed, 59 insertions(+)
>  create mode 100644 board/broadcom/bcmns3/fit/multi.its

Reviewed-by: Simon Glass 

Re: [PATCH 1/7] dm: button: add an uclass for button

[Simon Glass]

Hi Philippe,

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Add a new uclass for button that implements two functions:
> - button_get_by_label
> - button_get_status
>
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/Kconfig|  2 ++
>  drivers/Makefile   |  1 +
>  drivers/button/Kconfig | 12 ++
>  drivers/button/Makefile|  5 
>  drivers/button/button-uclass.c | 47 +
>  include/button.h   | 53 
> ++
>  include/dm/uclass-id.h |  1 +
>  7 files changed, 121 insertions(+)
>  create mode 100644 drivers/button/Kconfig
>  create mode 100644 drivers/button/Makefile
>  create mode 100644 drivers/button/button-uclass.c
>  create mode 100644 include/button.h

Reviewed-by: Simon Glass 

Please see below

>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index e34a227..3cb05aa 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -14,6 +14,8 @@ source "drivers/block/Kconfig"
>
>  source "drivers/bootcount/Kconfig"
>
> +source "drivers/button/Kconfig"
> +
>  source "drivers/cache/Kconfig"
>
>  source "drivers/clk/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 94e8c5d..727a713 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0+
>
> +obj-$(CONFIG_$(SPL_TPL_)BUTTON) += button/
>  obj-$(CONFIG_$(SPL_TPL_)CACHE) += cache/
>  obj-$(CONFIG_$(SPL_TPL_)CLK) += clk/
>  obj-$(CONFIG_$(SPL_TPL_)DM) += core/
> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> new file mode 100644
> index 000..8301858
> --- /dev/null
> +++ b/drivers/button/Kconfig
> @@ -0,0 +1,12 @@
> +menu "Button Support"
> +
> +config BUTTON
> +   bool "Enable button support"
> +   depends on DM
> +   help
> + Many boards have buttons which can be used to change behaviour 
> (reset, ...).
> + U-Boot provides a uclass API to implement this feature. Button 
> drivers
> + can provide access to board-specific buttons. Use of the device tree
> + for configuration is encouraged.
> +
> +endmenu
> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
> new file mode 100644
> index 000..0b4c128
> --- /dev/null
> +++ b/drivers/button/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (C) 2020 Philippe Reynes 
> +
> +obj-$(CONFIG_BUTTON) += button-uclass.o
> diff --git a/drivers/button/button-uclass.c b/drivers/button/button-uclass.c
> new file mode 100644
> index 000..d6cda78
> --- /dev/null
> +++ b/drivers/button/button-uclass.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Philippe Reynes 
> + *
> + * Based on led-uclass.c
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 

this one should go below common.h

> +
> +int button_get_by_label(const char *label, struct udevice **devp)
> +{
> +   struct udevice *dev;
> +   struct uclass *uc;
> +   int ret;
> +
> +   ret = uclass_get(UCLASS_BUTTON, &uc);
> +   if (ret)
> +   return ret;
> +   uclass_foreach_dev(dev, uc) {

uclass_id_foreach_dev

> +   struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +
> +   /* Ignore the top-level button node */
> +   if (uc_plat->label && !strcmp(label, uc_plat->label))
> +   return uclass_get_device_tail(dev, 0, devp);
> +   }
> +
> +   return -ENODEV;
> +}
> +
> +enum button_state_t button_get_state(struct udevice *dev)
> +{
> +   struct button_ops *ops = button_get_ops(dev);
> +
> +   if (!ops->get_state)
> +   return -ENOSYS;
> +
> +   return ops->get_state(dev);
> +}
> +
> +UCLASS_DRIVER(button) = {
> +   .id = UCLASS_BUTTON,
> +   .name   = "button",
> +   .per_device_platdata_auto_alloc_size = sizeof(struct button_uc_plat),
> +};
> diff --git a/include/button.h b/include/button.h
> new file mode 100644
> index 000..333fc23
> --- /dev/null
> +++ b/include/button.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2020 Philippe Reynes 
> + */
> +
> +#ifndef __BUTTON_H
> +#define __BUTTON_H
> +
> +/**
> + * struct button_uc_plat - Platform data the uclass stores about each device
> + *
> + * @label: Button label
> + */
> +struct button_uc_plat {
> +   const char *label;
> +};
> +

comment for this enum:

> +enum button_state_t {
> +   BUTTON_OFF = 0,
> +   BUTTON_ON = 1,
> +   BUTTON_COUNT,
> +};
> +
> +struct button_ops {
> +   /**
> +* get_state() - get the state of a button
> +*
> +* @dev:button device to change
> +* @return button state button_state_t, or -ve on error
> +*/
> +   enum button_state_t (*get_state)(struct udevice *dev);
> +};
> +
> +#define button_get_ops(dev)((struct

Re: [PATCH 7/7] test/py: add tests for the button commands

[Simon Glass]

Hi Philippe,

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Adds tests for the button commands.
>
> Signed-off-by: Philippe Reynes 
> ---
>  arch/sandbox/dts/test.dts| 14 ++
>  test/py/tests/test_button.py | 19 +++
>  2 files changed, 33 insertions(+)
>  create mode 100644 test/py/tests/test_button.py

Reviewed-by: Simon Glass 

But please also add a test/dm/button.c test that tests the uclass and
sandbox device.

Regards,
Simon

Re: [PATCH v2 06/14] board_f: m68k: Factor out m68k-specific bdinfo setup

[Simon Glass]

On Fri, 10 Jul 2020 at 04:25, Ovidiu Panait  wrote:
>
> Factor out m68k-specific bdinfo setup to arch_setup_bdinfo in
> arch/m68k/lib/bdinfo.c. Also, use if(IS_ENABLED()) instead of #ifdef where
> possible.
>
> Signed-off-by: Ovidiu Panait 
> ---
> v2 updates:
> - use "if (IS_ENABLED(CONFIG_SYS_HAS_SRAM))" instead of
>   "#ifdef CONFIG_SYS_SRAM_BASE"
>
>  arch/m68k/lib/bdinfo.c | 32 
>  common/board_f.c   | 14 --
>  2 files changed, 36 insertions(+), 10 deletions(-)

Reviewed-by: Simon Glass 

See below

>
> diff --git a/arch/m68k/lib/bdinfo.c b/arch/m68k/lib/bdinfo.c
> index 971c47c306..3257195add 100644
> --- a/arch/m68k/lib/bdinfo.c
> +++ b/arch/m68k/lib/bdinfo.c
> @@ -11,6 +11,38 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +int arch_setup_bdinfo(void)
> +{
> +   bd_t *bd = gd->bd;
> +
> +   /*
> +* Save local variables to board info struct
> +*/
> +   bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;/* start of memory */
> +   bd->bi_memsize = gd->ram_size;  /* size in bytes */
> +
> +   if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
> +   bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
> +   bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE;  /* size  of SRAM */
> +   }

I wonder if we can do better - can't we move the SDRAM code back to
the generic bdinfo.c ?


> +
> +   bd->bi_mbar_base = CONFIG_SYS_MBAR; /* base of internal registers */
> +
> +   bd->bi_intfreq = gd->cpu_clk;   /* Internal Freq, in Hz */
> +   bd->bi_busfreq = gd->bus_clk;   /* Bus Freq,  in Hz */
> +
> +   if (IS_ENABLED(CONFIG_PCI))
> +   bd->bi_pcifreq = gd->pci_clk;
> +
> +#if defined(CONFIG_EXTRA_CLOCK)
> +   bd->bi_inpfreq = gd->arch.inp_clk;  /* input Freq in Hz */
> +   bd->bi_vcofreq = gd->arch.vco_clk;  /* vco Freq in Hz */
> +   bd->bi_flbfreq = gd->arch.flb_clk;  /* flexbus Freq in Hz */
> +#endif
> +
> +   return 0;
> +}
> +
>  void arch_print_bdinfo(void)
>  {
> bd_t *bd = gd->bd;
> diff --git a/common/board_f.c b/common/board_f.c
> index e597749d2f..7d65879b93 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -602,7 +602,7 @@ __weak int arch_setup_bdinfo(void)
> return 0;
>  }
>
> -#if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
> defined(CONFIG_SH)
>  static int setup_board_part1(void)
>  {
> @@ -622,9 +622,6 @@ static int setup_board_part1(void)
>  #if defined(CONFIG_E500) || defined(CONFIG_MPC86xx)
> bd->bi_immr_base = CONFIG_SYS_IMMR; /* base  of IMMR register 
> */
>  #endif
> -#if defined(CONFIG_M68K)
> -   bd->bi_mbar_base = CONFIG_SYS_MBAR; /* base of internal registers 
> */
> -#endif
>  #if defined(CONFIG_MPC83xx)
> bd->bi_immrbar = CONFIG_SYS_IMMR;
>  #endif
> @@ -633,7 +630,7 @@ static int setup_board_part1(void)
>  }
>  #endif
>
> -#if defined(CONFIG_PPC) || defined(CONFIG_M68K)
> +#if defined(CONFIG_PPC)
>  static int setup_board_part2(void)
>  {
> bd_t *bd = gd->bd;
> @@ -646,9 +643,6 @@ static int setup_board_part2(void)
> bd->bi_sccfreq = gd->arch.scc_clk;
> bd->bi_vco = gd->arch.vco_out;
>  #endif /* CONFIG_CPM2 */
> -#if defined(CONFIG_M68K) && defined(CONFIG_PCI)
> -   bd->bi_pcifreq = gd->pci_clk;
> -#endif
>  #if defined(CONFIG_EXTRA_CLOCK)
> bd->bi_inpfreq = gd->arch.inp_clk;  /* input Freq in Hz */
> bd->bi_vcofreq = gd->arch.vco_clk;  /* vco Freq in Hz */
> @@ -980,11 +974,11 @@ static const init_fnc_t init_sequence_f[] = {
> dram_init_banksize,
> show_dram_config,
> arch_setup_bdinfo,
> -#if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
> defined(CONFIG_SH)
> setup_board_part1,
>  #endif
> -#if defined(CONFIG_PPC) || defined(CONFIG_M68K)
> +#if defined(CONFIG_PPC)
> INIT_FUNC_WATCHDOG_RESET
> setup_board_part2,
>  #endif
> --
> 2.17.1
>

Re: [PATCH 1/3] ARM: rmobile: Switch back to fdtdec_setup_memory/banksize_fdt()

[Simon Glass]

On Fri, 10 Jul 2020 at 05:16, Michal Simek  wrote:
>
> The commit 361377dbdbc9 ("ARM: rmobile: Merge prior-stage firmware DT
> fragment into U-Boot DT on Gen3") reverted changes introduced by commit
> 175f5027345c ("ARM: renesas: Configure DRAM size from ATF DT fragment")
> that's why there is no reason to use functions with _fdt() suffix because
> parameter is gd->fdt_blob as is already for functions without _fdt()
> suffix.
>
> Signed-off-by: Michal Simek 
> ---
>
>  board/renesas/rcar-common/common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH 4/7] sandbox: dtsi: add buttons

[Simon Glass]

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Adds two buttons on sandbox so button framework may be tested.
>
> Signed-off-by: Philippe Reynes 
> ---
>  arch/sandbox/dts/sandbox.dtsi | 14 ++
>  1 file changed, 14 insertions(+)
>

Reviewed-by: Simon Glass 

But don't you want this in test.dtsi too if it is for testing?

Re: [PATCH 2/7] dm: button: add a driver for button driven by gpio

[Simon Glass]

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Add a simple driver which allows use of buttons attached to GPIOs.
>
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/button/Kconfig   |  10 
>  drivers/button/Makefile  |   1 +
>  drivers/button/button-gpio.c | 111 
> +++
>  3 files changed, 122 insertions(+)
>  create mode 100644 drivers/button/button-gpio.c

Reviewed-by: Simon Glass 


>
> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> index 8301858..7de1a97 100644
> --- a/drivers/button/Kconfig
> +++ b/drivers/button/Kconfig
> @@ -9,4 +9,14 @@ config BUTTON
>   can provide access to board-specific buttons. Use of the device tree
>   for configuration is encouraged.
>
> +config BUTTON_GPIO
> +   bool "Button gpio"
> +   depends on BUTTON
> +   default n

not needed

> +   help
> + Enable support for buttons which are connected to GPIO lines. These
> + GPIOs may be on the SoC or some other device which provides GPIOs.
> + The GPIO driver must used driver model. Buttons are configured using
> + the device tree.
> +
>  endmenu
> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
> index 0b4c128..fcc10eb 100644
> --- a/drivers/button/Makefile
> +++ b/drivers/button/Makefile
> @@ -3,3 +3,4 @@
>  # Copyright (C) 2020 Philippe Reynes 
>
>  obj-$(CONFIG_BUTTON) += button-uclass.o
> +obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
> diff --git a/drivers/button/button-gpio.c b/drivers/button/button-gpio.c
> new file mode 100644
> index 000..6ad0094
> --- /dev/null
> +++ b/drivers/button/button-gpio.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Philippe Reynes 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please fix order
https://www.denx.de/wiki/U-Boot/CodingStyle

> +
> +struct button_gpio_priv {
> +   struct gpio_desc gpio;
> +};
> +
> +static enum button_state_t button_gpio_get_state(struct udevice *dev)
> +{
> +   struct button_gpio_priv *priv = dev_get_priv(dev);
> +   int ret;
> +
> +   if (!dm_gpio_is_valid(&priv->gpio))
> +   return -EREMOTEIO;
> +   ret = dm_gpio_get_value(&priv->gpio);
> +   if (ret < 0)
> +   return ret;
> +
> +   return ret ? BUTTON_ON : BUTTON_OFF;
> +}
> +
> +static int button_gpio_probe(struct udevice *dev)
> +{
> +   struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +   struct button_gpio_priv *priv = dev_get_priv(dev);
> +   int ret;
> +
> +   /* Ignore the top-level button node */
> +   if (!uc_plat->label)
> +   return 0;
> +
> +   ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, GPIOD_IS_IN);
> +   if (ret)
> +   return ret;
> +
> +   return 0;
> +}
> +
> +static int button_gpio_remove(struct udevice *dev)
> +{
> +   /*
> +* The GPIO driver may have already been removed. We will need to
> +* address this more generally.

What is needed here?

> +*/
> +   if (IS_ENABLED(CONFIG_SANDBOX)) {
> +   struct button_gpio_priv *priv = dev_get_priv(dev);
> +
> +   if (dm_gpio_is_valid(&priv->gpio))
> +   dm_gpio_free(dev, &priv->gpio);
> +   }
> +
> +   return 0;
> +}
> +

[..]

Re: [PATCH v2 1/3] lib: fdt: Introduce fdtdec_setup_mem_size_base_lowest()

[Simon Glass]

On Fri, 10 Jul 2020 at 06:41, Michal Simek  wrote:
>
> New function should be called from board dram_init() because it initialized
> gd->ram_base/ram_size. It finds the lowest available memory.
>
> On systems with multiple memory nodes finding out the first memory node by
> fdtdec_setup_mem_size_base() is not enough because this memory can be above
> actual U-Boot VA mapping. Currently only mapping till 39bit is supported
> (Full 44bit mapping was removed by commit 7985cdf74b28 ("arm64: Remove
> non-full-va map code")).
> If DT starts with the first memory node above 39bit address then system can
> be unpredictable.
>
> The function is available only when multiple memory bank support is
> enabled.
>
> Calling fdtdec_setup_memory_banksize() from dram_init() is not possible
> because fdtdec_setup_memory_banksize() is saving dram information to bd
> structure which is placed on stack but not initialized at this time. Also
> stack is placed at location setup in dram_init().
>
> Signed-off-by: Michal Simek 
> ---
>
> Changes in v2:
> - Separate Xilinx change from generic patch
>
>  include/fdtdec.h | 17 +
>  lib/fdtdec.c | 45 +
>  2 files changed, 62 insertions(+)
>

Reviewed-by: Simon Glass 

Re: [PATCH 5/7] sandbox64: enable button

[Simon Glass]

On Mon, 13 Jul 2020 at 06:56, Philippe Reynes
 wrote:
>
> Enable the support of button (driver and command) on sandbox64.
>
> Signed-off-by: Philippe Reynes 
> ---
>  configs/sandbox64_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass 

Re: [PATCH v2 09/14] board_f: mips: Factor out mips-specific bdinfo setup

[Simon Glass]

On Fri, 10 Jul 2020 at 04:25, Ovidiu Panait  wrote:
>
> Factor out mips-specific bdinfo setup from generic init sequence to
> arch_setup_bdinfo in arch/mips/lib/boot.c. Also, use if(IS_ENABLED())
> instead of #ifdef where possible.
>
> Signed-off-by: Ovidiu Panait 
> ---
> v2 updates:
> - use "if (IS_ENABLED(CONFIG_SYS_HAS_SRAM))" instead of
>   "#ifdef CONFIG_SYS_SRAM_BASE"
>
>  arch/mips/lib/boot.c | 18 ++
>  common/board_f.c | 25 +
>  2 files changed, 19 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH v2 08/14] board_f: sh: Factor out sh-specific bdinfo setup

[Simon Glass]

On Fri, 10 Jul 2020 at 04:25, Ovidiu Panait  wrote:
>
> Factor out sh-specific bdinfo setup from generic init sequence to
> arch_setup_bdinfo in arch/sh/lib/board.c. Also, use if(IS_ENABLED())
> instead of #ifdef where possible.
>
> Signed-off-by: Ovidiu Panait 
> ---
> v2 updates:
> - use "if (IS_ENABLED(CONFIG_SYS_HAS_SRAM))" instead of
>   "#ifdef CONFIG_SYS_SRAM_BASE"
>
>  arch/sh/lib/board.c | 18 ++
>  common/board_f.c|  4 ++--
>  2 files changed, 20 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

Same comment here.

Re: [PATCH v4 1/2] drivers: tee: broadcom: add optee based bnxt fw load driver

[Simon Glass]

On Fri, 10 Jul 2020 at 04:51, Rayagonda Kokatanur
 wrote:
>
> From: Vikas Gupta 
>
> Add optee based bnxt fw load driver.
> bnxt is Broadcom NetXtreme controller Ethernet card.
> This driver is used to load bnxt firmware binary using OpTEE.
>
> Signed-off-by: Vikas Gupta 
> Signed-off-by: Rayagonda Kokatanur 
> ---
> Changes from v3:
>  -Address review comments from Simon,
>   Rearrange code and remove while loop,
>   Add comments for function.
>
> Changes from v2:
>  -Address review comments from Simon,
>   Remove own return code and use standard error code.
>   Take out common lines from different functions and move them
>   into common static function.
>   Remove include  as its not required.
>   Move functions with printf from header file into c file.
>
>  drivers/tee/Kconfig|   1 +
>  drivers/tee/Makefile   |   1 +
>  drivers/tee/broadcom/Kconfig   |   7 ++
>  drivers/tee/broadcom/Makefile  |   3 +
>  drivers/tee/broadcom/chimp_optee.c | 180 +
>  include/broadcom/chimp.h   |  43 +++
>  6 files changed, 235 insertions(+)
>  create mode 100644 drivers/tee/broadcom/Kconfig
>  create mode 100644 drivers/tee/broadcom/Makefile
>  create mode 100644 drivers/tee/broadcom/chimp_optee.c
>  create mode 100644 include/broadcom/chimp.h

Reviewed-by: Simon Glass 

Blank line before last 'return' please.

Re: [PATCH] serial: Fix SIFIVE debug serial dependency

[Tom Rini]

On Tue, Jul 14, 2020 at 07:05:54PM -0600, Simon Glass wrote:
> On Fri, 10 Jul 2020 at 00:21, Michal Simek  wrote:
> >
> > Hi Simon,
> >
> > On 10. 07. 20 2:35, Simon Glass wrote:
> > > Hi Michal,
> > >
> > > On Thu, 9 Jul 2020 at 08:17, Michal Simek  wrote:
> > >>
> > >> The commit 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for
> > >> debug consoles") has added incorrect dependency for SIFIVE debug uart 
> > >> which
> > >> should depend on SIFIVE driver instead of PL01x.
> > >
> > > Does that mean this should have a Fixes: tag?
> >
> > TBH I had it there but removed it because commit is pointing to it
> > anyway. And also I am not aware about any stable process in place.
> > Or is there any process around that people are looking for fixed tags
> > and picking them to any u-boot stable tree?
> 
> +Tom Rini who may know

We do fixes tags because it's good history.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 07/14] board_f: ppc: Factor out ppc-specific bdinfo setup

[Simon Glass]

On Fri, 10 Jul 2020 at 04:25, Ovidiu Panait  wrote:
>
> Factor out ppc-specific bdinfo setup from generic init sequence to
> arch_setup_bdinfo in arch/powerpc/lib/bdinfo.c. Also, use if(IS_ENABLED())
> instead of #ifdef where possible.
>
> Signed-off-by: Ovidiu Panait 
> ---
> v2 updates:
> - use "if (IS_ENABLED(CONFIG_SYS_HAS_SRAM))" instead of
>   "#ifdef CONFIG_SYS_SRAM_BASE"
>
>  arch/powerpc/lib/bdinfo.c | 42 +++
>  common/board_f.c  | 39 ++--
>  2 files changed, 44 insertions(+), 37 deletions(-)
>

Reviewed-by: Simon Glass 

Re: [PATCH v3 04/17] common: update: add a generic interface for FIT image

[AKASHI Takahiro]

Heinrich,

On Fri, Jul 10, 2020 at 06:24:45PM +0200, Heinrich Schuchardt wrote:
> On 10.07.20 03:25, AKASHI Takahiro wrote:
> > The main purpose of this patch is to separate a generic interface for
> > updating firmware using DFU drivers from "auto-update" via tftp.
> >
> > This function will also be used in implementing UEFI capsule update
> > in a later commit.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  common/Kconfig  | 16 +++
> >  common/Makefile |  2 +-
> >  common/update.c | 75 +++--
> >  include/image.h | 12 
> >  4 files changed, 102 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 2d86dd7e63c6..a51d2a7b9d2a 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -985,9 +985,16 @@ endmenu
> >
> >  menu "Update support"
> >
> > +config UPDATE_COMMON
> > +   bool
> > +   default n
> > +   select DFU_ALT
> > +
> >  config UPDATE_TFTP
> > bool "Auto-update using fitImage via TFTP"
> > depends on FIT
> > +   depends on DFU
> > +   select UPDATE_COMMON
> > help
> >   This option allows performing update of NOR with data in fitImage
> >   sent via TFTP boot.
> > @@ -1002,6 +1009,15 @@ config UPDATE_TFTP_MSEC_MAX
> > default 100
> > depends on UPDATE_TFTP
> >
> > +config UPDATE_FIT
> > +   bool "Firmware update using fitImage"
> > +   depends on FIT
> > +   depends on DFU
> > +   select UPDATE_COMMON
> > +   help
> > + This option allows performing update of DFU-capable storage with
> > + data in fitImage.
> > +
> >  config ANDROID_AB
> > bool "Android A/B updates"
> > default n
> > diff --git a/common/Makefile b/common/Makefile
> > index 2e7a090588d9..c238db8108e7 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -53,7 +53,7 @@ obj-$(CONFIG_LCD_ROTATION) += lcd_console_rotation.o
> >  obj-$(CONFIG_LCD_DT_SIMPLEFB) += lcd_simplefb.o
> >  obj-$(CONFIG_LYNXKDI) += lynxkdi.o
> >  obj-$(CONFIG_MENU) += menu.o
> > -obj-$(CONFIG_UPDATE_TFTP) += update.o
> > +obj-$(CONFIG_UPDATE_COMMON) += update.o
> >  obj-$(CONFIG_DFU_TFTP) += update.o
> >  obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
> >  obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
> > diff --git a/common/update.c b/common/update.c
> > index 3547dc68bf7c..a8151383fae7 100644
> > --- a/common/update.c
> > +++ b/common/update.c
> > @@ -24,6 +24,7 @@
> >  #include 
> >  #include 
> >
> > +#ifdef CONFIG_UPDATE_TFTP
> >  /* env variable holding the location of the update file */
> >  #define UPDATE_FILE_ENV"updatefile"
> >
> > @@ -211,6 +212,7 @@ static int update_flash(ulong addr_source, ulong 
> > addr_first, ulong size)
> > return 1;
> >  #endif
> >  }
> > +#endif /* CONFIG_UPDATE_TFTP */
> >
> >  static int update_fit_getparams(const void *fit, int noffset, ulong *addr,
> > ulong *fladdr, ulong *size)
> > @@ -228,6 +230,7 @@ static int update_fit_getparams(const void *fit, int 
> > noffset, ulong *addr,
> > return 0;
> >  }
> >
> > +#ifdef CONFIG_UPDATE_TFTP
> >  int update_tftp(ulong addr, char *interface, char *devstring)
> >  {
> > char *filename, *env_addr, *fit_image_name;
> > @@ -322,8 +325,9 @@ got_update_file:
> > }
> > } else if (fit_image_check_type(fit, noffset,
> > IH_TYPE_FIRMWARE)) {
> > -   ret = dfu_tftp_write(fit_image_name, update_addr,
> > -update_size, interface, devstring);
> > +   ret = dfu_write_by_name(fit_image_name, update_addr,
> > +   update_size, interface,
> > +   devstring);
> > if (ret)
> > return ret;
> > }
> > @@ -333,3 +337,70 @@ next_node:
> >
> > return ret;
> >  }
> > +#endif
> > +
> > +#ifdef CONFIG_UPDATE_FIT
> > +/**
> > + * fit_update - update storage with FIT image
> > + * @fit:   Pointer to FIT image
> > + *
> > + * Update firmware on storage using FIT image as input.
> > + * The storage area to be update will be identified by the name
> > + * in FIT and matching it to "dfu_alt_info" variable.
> > + *
> > + * Return:  0 - on success, non-zero - otherwise
> > + */
> > +int fit_update(const void *fit)
> 
> With patch
> 
> cmd: drop fitupd command
> https://patchwork.ozlabs.org/project/uboot/patch/20200617090903.9268-1-xypron.g...@gmx.de/
> 
> the fitupd command is removed.
> 
> Lukasz has to put the patch into a pull request, yet
> 
> Once that patch has been inserted only the automatic fit updates will
> use code in common/update.c. This code is not called by the 'dfu tftp'
> command.

?
In cmd/dfu.c,
===8<===
#ifdef CONFIG_DFU_OVER_TFTP
if (!strcmp(argv[1], "tftp"))
return update_tftp(value, interface, devstring);
#endif
===>8===

> The function fit_updat

Re: [PATCH] test: use virt-make-fs to build image

[AKASHI Takahiro]

Heinrich,

On Tue, Jul 14, 2020 at 01:18:04AM +0200, Heinrich Schuchardt wrote:
> Am 14. Juli 2020 01:02:07 MESZ schrieb AKASHI Takahiro 
> :
> >Heinrich,
> >
> >On Tue, Jul 14, 2020 at 12:15:34AM +0200, Heinrich Schuchardt wrote:
> >> Avoid sudo for test/py/tests/test_efi_secboot by using virt-make-fs.
> >
> >Have you read this?
> >https://lists.denx.de/pipermail/u-boot/2020-July/419345.html
> >
> >-Takahiro Akashi
> 
> Just put chmod 644 /boot/vmlinu* into your update-initramfs hook directory 
> /etc/initramfs-tools and complain to Ubuntu maintainers about them breaking 
> their own packages.

If this solves all the issues, that's fine.
But if you require this extra step to run the test, you should
describe it explicitly somewhere. Otherwise, people can get confused.

I believe that it would be still useful to have a fall-back
method of "sudo" version.

> Hook scripts are described here: 
> http://manpages.ubuntu.com/manpages/xenial/man8/initramfs-tools.8.html
> 
> >
> >> Signed-off-by: Heinrich Schuchardt 
> >> ---
> >>  test/py/tests/test_efi_secboot/conftest.py | 27
> >--
> >>  test/py/tests/test_efi_secboot/defs.py |  7 --
> >>  2 files changed, 4 insertions(+), 30 deletions(-)
> >> 
> >> diff --git a/test/py/tests/test_efi_secboot/conftest.py
> >b/test/py/tests/test_efi_secboot/conftest.py
> >> index 71ef723e59..c6709700a8 100644
> >> --- a/test/py/tests/test_efi_secboot/conftest.py
> >> +++ b/test/py/tests/test_efi_secboot/conftest.py
> >> @@ -38,34 +38,15 @@ def efi_boot_env(request, u_boot_config):
> >> 
> >>  image_path = u_boot_config.persistent_data_dir
> >>  image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME
> >> -image_size = EFI_SECBOOT_IMAGE_SIZE
> >> -part_size = EFI_SECBOOT_PART_SIZE
> >> -fs_type = EFI_SECBOOT_FS_TYPE
> >> 
> >>  if HELLO_PATH == '':
> >>  HELLO_PATH = u_boot_config.build_dir +
> >'/lib/efi_loader/helloworld.efi'
> >> 
> >>  try:
> >> -mnt_point = u_boot_config.persistent_data_dir +
> >'/mnt_efisecure'
> >> +mnt_point = u_boot_config.build_dir + '/mnt_efisecure'

Please use persistent_data_dir here as a work directory.
'build_dir' is for U-Boot build.

-Takahiro Akashi


> >> +check_call('rm -rf {}'.format(mnt_point), shell=True)
> >>  check_call('mkdir -p {}'.format(mnt_point), shell=True)
> >> 
> >> -# create a disk/partition
> >> -check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> >> -   % (image_path, image_size), shell=True)
> >> -check_call('sgdisk %s -n 1:0:+%dMiB'
> >> -   % (image_path, part_size), shell=True)
> >> -# create a file system
> >> -check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d'
> >> -   % (image_path, part_size), shell=True)
> >> -check_call('mkfs -t %s %s.tmp' % (fs_type, image_path),
> >shell=True)
> >> -check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d
> >conv=notrunc'
> >> -   % (image_path, image_path, 1), shell=True)
> >> -check_call('rm %s.tmp' % image_path, shell=True)
> >> -loop_dev = check_output('sudo losetup -o 1MiB --sizelimit
> >%dMiB --show -f %s | tr -d "\n"'
> >> -% (part_size, image_path),
> >shell=True).decode()
> >> -check_output('sudo mount -t %s -o umask=000 %s %s'
> >> - % (fs_type, loop_dev, mnt_point), shell=True)
> >> -
> >>  # suffix
> >>  # *.key: RSA private key in PEM
> >>  # *.crt: X509 certificate (self-signed) in PEM
> >> @@ -145,8 +126,8 @@ def efi_boot_env(request, u_boot_config):
> >> % (mnt_point, EFITOOLS_PATH),
> >> shell=True)
> >> 
> >> -check_call('sudo umount %s' % loop_dev, shell=True)
> >> -check_call('sudo losetup -d %s' % loop_dev, shell=True)
> >> +check_call('virt-make-fs --partition=gpt --size=+1M
> >--type=vfat {} {}'.format(mnt_point, image_path), shell=True)
> >> +check_call('rm -rf {}'.format(mnt_point), shell=True)
> >> 
> >>  except CalledProcessError as exception:
> >>  pytest.skip('Setup failed: %s' % exception.cmd)
> >> diff --git a/test/py/tests/test_efi_secboot/defs.py
> >b/test/py/tests/test_efi_secboot/defs.py
> >> index 099f453979..ba6b9f391e 100644
> >> --- a/test/py/tests/test_efi_secboot/defs.py
> >> +++ b/test/py/tests/test_efi_secboot/defs.py
> >> @@ -3,13 +3,6 @@
> >>  # Disk image name
> >>  EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
> >> 
> >> -# Size in MiB
> >> -EFI_SECBOOT_IMAGE_SIZE = 16
> >> -EFI_SECBOOT_PART_SIZE = 8
> >> -
> >> -# Partition file system type
> >> -EFI_SECBOOT_FS_TYPE = 'vfat'
> >> -
> >>  # Owner guid
> >>  GUID = '----123456789abc'
> >> 
> >> --
> >> 2.27.0
> >> 
> 

Re: [PATCH v3 06/17] efi_loader: add option to initialise EFI subsystem early

[AKASHI Takahiro]

Heinrich,

On Fri, Jul 10, 2020 at 06:25:34PM +0200, Heinrich Schuchardt wrote:
> On 10.07.20 03:25, AKASHI Takahiro wrote:
> > If this option, CONFIG_EFI_SETUP_EARLY, is enabled, the initialisation
> > of UEFI subsystem will be done as part of U-Boot initialisation.
> >
> > Please note that this option won't be enabled explicitly by users,
> > instead, should be enabled implicitly by other configuration options.
> >
> > Specifically, this feature will be utilised in implementing capsule-on-disk
> > feature.
> 
> This breaks access to block devices in UEFI. You cannot do the
> initialization before the initialization of block devices which
> currently is not done automatically but requires commands like 'scsi
> scan' in a script or entered manually.

I know that, but I'm also sure there are some devices detected before
executing such kind of commands.

The reason that I added this configuration is to give users alternative
choices depending on their systems and 'needs'.

> It might make sense to scan all block devices automatically instead of
> requiring command entry.

I don't object to this idea, but it can break the compatibility
with existing config_distro_bootcmd framework.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  common/board_r.c   | 6 ++
> >  lib/efi_loader/Kconfig | 4 
> >  2 files changed, 10 insertions(+)
> >
> > diff --git a/common/board_r.c b/common/board_r.c
> > index fa57fa9b6993..dcb8c6f79d2f 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -68,6 +68,9 @@
> >  #if defined(CONFIG_GPIO_HOG)
> >  #include 
> >  #endif
> > +#ifdef CONFIG_EFI_SETUP_EARLY
> > +#include 
> > +#endif
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -858,6 +861,9 @@ static init_fnc_t init_sequence_r[] = {
> >  #endif
> >  #if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE)
> > blkcache_init,
> > +#endif
> > +#ifdef CONFIG_EFI_SETUP_EARLY
> > +   (init_fnc_t)efi_init_obj_list,
> >  #endif
> > run_main_loop,
> >  };
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 6c9df3a76763..f0a30a43bc48 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -27,6 +27,10 @@ config EFI_LOADER
> >
> >  if EFI_LOADER
> >
> > +config EFI_SETUP_EARLY
> > +   bool
> > +   default n
> > +
> >  config EFI_GET_TIME
> > bool "GetTime() runtime service"
> > depends on DM_RTC
> >
> 

Re: [PATCH v3 05/17] dfu: export dfu_list

[AKASHI Takahiro]

Heinrich,

On Fri, Jul 10, 2020 at 06:25:16PM +0200, Heinrich Schuchardt wrote:
> On 10.07.20 03:25, AKASHI Takahiro wrote:
> > This variable will be utilized to enumerate all dfu entities
> > for UEFI capsule firmware update in a later commit.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  drivers/dfu/dfu.c | 2 +-
> >  include/dfu.h | 3 +++
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> > index a298c2c43999..501a60b34400 100644
> > --- a/drivers/dfu/dfu.c
> > +++ b/drivers/dfu/dfu.c
> > @@ -18,7 +18,7 @@
> >  #include 
> >  #include 
> >
> > -static LIST_HEAD(dfu_list);
> > +LIST_HEAD(dfu_list);
> >  static int dfu_alt_num;
> >  static int alt_num_cnt;
> >  static struct hash_algo *dfu_hash_algo;
> > diff --git a/include/dfu.h b/include/dfu.h
> > index 94b0a9e68317..e9af9503d685 100644
> > --- a/include/dfu.h
> > +++ b/include/dfu.h
> > @@ -158,6 +158,9 @@ struct dfu_entity {
> > unsigned int inited:1;
> >  };
> >
> > +struct list_head;
> > +extern struct list_head dfu_list;
> > +
> >  #ifdef CONFIG_SET_DFU_ALT_INFO
> >  /**
> >   * set_dfu_alt_info() - set dfu_alt_info environment variable
> >
> 
> Why would you need to export this list if the current implementation of
> the dfu command does not need to export it?

Because there is a new 'need', that is enumerating all the definitions
of dfu targets for users, i.e. UEFI subsystem.

> Please, use the existing DFU API functions instead of accessing a
> private member of the implementation.

There is no appropriate API at this moment.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> 

Re: [PATCH v3 02/17] dfu: add a hidden reverse-dependency on UPDATE_TFTP

[AKASHI Takahiro]

Heinrich,

On Fri, Jul 10, 2020 at 07:21:14AM +0200, Heinrich Schuchardt wrote:
> On 7/10/20 3:25 AM, AKASHI Takahiro wrote:
> > DFU_OVER_TFTP support on "dfu" command relies on update_tftp()
> > being available. Just explicitly add this dependency.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  drivers/dfu/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> > index 5d45d7d7c2d7..cafb6a34090e 100644
> > --- a/drivers/dfu/Kconfig
> > +++ b/drivers/dfu/Kconfig
> > @@ -12,6 +12,7 @@ config DFU_OVER_USB
> >  config DFU_OVER_TFTP
> > bool
> > depends on NET
> > +   select UPDATE_TFTP
> 
> UPDATE_TFTP depends on FIT while DFU_OVER_TFTP does not depend on FIT.
> 
> So selecting UPDATE_TFTP without FIT creates an invalid configuration:

Please read the code in common/update.c carefully.
Whether my patch is applied or not, update_tftp()
relies on FIT (and helper functions.)

So 'selecting UPDATE_TFTP without FIT' is just invalid.

> Symbol: UPDATE_TFTP [=y]
> Type  : bool
> Prompt: Auto-update using fitImage via TFTP
>   Location:
> (1) -> Update support
>   Defined at common/Kconfig:1017
>   Depends on: FIT [=n]
>   Selected by [y]:
>   - DFU_OVER_TFTP [=y] && NET [=y]
> 
> WARNING: unmet direct dependencies detected for UPDATE_TFTP
>   Depends on [n]: FIT [=n]
>   Selected by [y]:
>   - DFU_OVER_TFTP [=y] && NET [=y]
> 
> common/update.c: In function ‘update_fit_getparams’:
> common/update.c:220:6: error: implicit declaration of function
> ‘fit_image_get_data’; did you mean ‘image_get_data’?
> [-Werror=implicit-function-declaration]
>   220 |  if (fit_image_get_data(fit, noffset, &data, (size_t *)size))
> 
> The whole UEFI capsule update development should not depend on NET and
> hence not on UPDATE_TFTP.

Please read my succeeding patches carefully.
With all the patches applied, I guarantee that your assertion above is valid.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> >  if DFU
> >  config DFU_TFTP
> >
> 

Re: [PATCH v3 03/17] dfu: rename dfu_tftp_write() to dfu_write_by_name()

[AKASHI Takahiro]

Heinrich,

On Fri, Jul 10, 2020 at 07:45:30AM +0200, Heinrich Schuchardt wrote:
> On 7/10/20 3:25 AM, AKASHI Takahiro wrote:
> > This function is essentially independent from tffp, and will also be
> > utilised in implementing UEFI capsule update in a later commit.
> > So just give it a more generic name.
> > In addition, a new configuration option, CONFIG_DFU_ALT, was introduced
> 
> _ALT does not convey any meaning to me. Why don't you simply move the
> function to drivers/dfu/dfu.c?

Please read my succeeding patches carefully.
I added a variant function later.

> If it is not needed the linker will drop it.

CONFIG_DFU_ALT is NOT a user-selectable configuration.
So unless it is really necessary, it won't compile 'automatically'.
We don't have to rely on linkers here.

> dfu_write_buffer() might be a good name for the function.

See my comment above.
There is a good reason for the name.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > so that the file will be compiled with different options, particularly
> > one added in a later commit.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  drivers/dfu/Kconfig   |  5 +
> >  drivers/dfu/Makefile  |  2 +-
> >  drivers/dfu/{dfu_tftp.c => dfu_alt.c} | 17 --
> >  include/dfu.h | 32 +--
> >  4 files changed, 37 insertions(+), 19 deletions(-)
> >  rename drivers/dfu/{dfu_tftp.c => dfu_alt.c} (67%)
> >
> > diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> > index cafb6a34090e..1bddaef45532 100644
> > --- a/drivers/dfu/Kconfig
> > +++ b/drivers/dfu/Kconfig
> > @@ -15,8 +15,13 @@ config DFU_OVER_TFTP
> > select UPDATE_TFTP
> >
> >  if DFU
> > +config DFU_ALT
> > +   bool
> > +   default n
> > +
> >  config DFU_TFTP
> > bool "DFU via TFTP"
> > +   select DFU_ALT
> > select DFU_OVER_TFTP
> > help
> >   This option allows performing update of DFU-managed medium with data
> > diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
> > index 0d7925c083ef..cc7de1d3ed9b 100644
> > --- a/drivers/dfu/Makefile
> > +++ b/drivers/dfu/Makefile
> > @@ -9,5 +9,5 @@ obj-$(CONFIG_$(SPL_)DFU_MTD) += dfu_mtd.o
> >  obj-$(CONFIG_$(SPL_)DFU_NAND) += dfu_nand.o
> >  obj-$(CONFIG_$(SPL_)DFU_RAM) += dfu_ram.o
> >  obj-$(CONFIG_$(SPL_)DFU_SF) += dfu_sf.o
> > -obj-$(CONFIG_$(SPL_)DFU_TFTP) += dfu_tftp.o
> > +obj-$(CONFIG_$(SPL_)DFU_ALT) += dfu_alt.o
> >  obj-$(CONFIG_$(SPL_)DFU_VIRT) += dfu_virt.o
> > diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_alt.c
> > similarity index 67%
> > rename from drivers/dfu/dfu_tftp.c
> > rename to drivers/dfu/dfu_alt.c
> > index ffae4bb54f80..5b1b13d7170d 100644
> > --- a/drivers/dfu/dfu_tftp.c
> > +++ b/drivers/dfu/dfu_alt.c
> > @@ -10,8 +10,21 @@
> >  #include 
> >  #include 
> >
> > -int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int 
> > len,
> > -  char *interface, char *devstring)
> > +/**
> > + * dfu_write_by_name() - write data to DFU medium
> > + * @dfu_entity_name:Name of DFU entity to write
> > + * @addr:   Address of data buffer to write
> > + * @len:Number of bytes
> > + * @interface:  Destination DFU medium (e.g. "mmc")
> > + * @devstring:  Instance number of destination DFU medium (e.g. 
> > "1")
> > + *
> > + * This function is storing data received on DFU supported medium which
> > + * is specified by @dfu_entity_name.
> > + *
> > + * Return:  0 - on success, error code - otherwise
> > + */
> > +int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
> > + unsigned int len, char *interface, char *devstring)
> >  {
> > char *s, *sb;
> > int alt_setting_num, ret;
> > diff --git a/include/dfu.h b/include/dfu.h
> > index 6fa450593605..94b0a9e68317 100644
> > --- a/include/dfu.h
> > +++ b/include/dfu.h
> > @@ -494,27 +494,27 @@ static inline int dfu_fill_entity_virt(struct 
> > dfu_entity *dfu, char *devstr,
> >  #endif
> >
> >  /**
> > - * dfu_tftp_write() - write TFTP data to DFU medium
> > + * dfu_write_by_name() - write data to DFU medium
> > + * @dfu_entity_name:   Name of DFU entity to write
> > + * @addr:  Address of data buffer to write
> > + * @len:   Number of bytes
> > + * @interface: Destination DFU medium (e.g. "mmc")
> > + * @devstring: Instance number of destination DFU medium (e.g. 
> > "1")
> >   *
> > - * This function is storing data received via TFTP on DFU supported medium.
> > + * This function is storing data received on DFU supported medium which
> > + * is specified by @dfu_entity_name.
> >   *
> > - * @dfu_entity_name:   name of DFU entity to write
> > - * @addr:  address of data buffer to write
> > - * @len:   number of bytes
> > - * @interface: destination DFU medium (e.g. "mmc")
> > - * @devstring: instance number of destination DFU medium (e.g. 
> > "1")
> > - *
> > - * R

Re: [PATCH] serial: Fix SIFIVE debug serial dependency

[Michal Simek]

On 15. 07. 20 3:10, Tom Rini wrote:
> On Tue, Jul 14, 2020 at 07:05:54PM -0600, Simon Glass wrote:
>> On Fri, 10 Jul 2020 at 00:21, Michal Simek  wrote:
>>>
>>> Hi Simon,
>>>
>>> On 10. 07. 20 2:35, Simon Glass wrote:
 Hi Michal,

 On Thu, 9 Jul 2020 at 08:17, Michal Simek  wrote:
>
> The commit 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for
> debug consoles") has added incorrect dependency for SIFIVE debug uart 
> which
> should depend on SIFIVE driver instead of PL01x.

 Does that mean this should have a Fixes: tag?
>>>
>>> TBH I had it there but removed it because commit is pointing to it
>>> anyway. And also I am not aware about any stable process in place.
>>> Or is there any process around that people are looking for fixed tags
>>> and picking them to any u-boot stable tree?
>>
>> +Tom Rini who may know
> 
> We do fixes tags because it's good history.
> 

ok. Thx.
M

Re: Using gerrit or github for review?

[Michael Nazzareno Trimarchi]

Hi all

On Wed, Jul 15, 2020 at 3:08 AM Simon Glass  wrote:
>
> Hi,
>
> On Tue, 14 Jul 2020 at 14:06, Karsten Merker  wrote:
> >
> > On Mon, Jul 13, 2020 at 06:05:42PM -0400, Corey Clayton wrote:
> > > On Mon, Jul 13, 2020 at 12:25:42PM -0600, Simon Glass wrote:
> > >
> > > > At present U-Boot uses the mailing list for patch review. What do
> > > > people think about trying out geritt or github for this? I'd be
> > > > willing to do a trial with the -dm mailing list.
> > >
> > > This is both my first message to the mailing list and my first
> > > email sent using mutt.  I'm hoping to eventually participate
> > > with patches and reviews but the mailing-list-driven
> > > developement model has been a barrier for myself an probably
> > > many others.  I'm slowly trying to climb over it now but some
> > > will never find the time.  Perhaps a good question is: How long
> > > does it take to learn the mailing-list workflow vs the github
> > > workflow?
> > >
> > > If u-boot was using github, I would have contributed long ago
> > > and I suspect there are others in the same bucket.  Thats my
> > > perspective at least :)
> >
> > Hello,
> >
> > to provide a different perspective: if U-Boot would have done
> > everything inside github instead of using its traditional
> > mailinglist-based workflow, I would never have contributed to
> > U-Boot, and moving everything from the mailinglist to github
> > would make any future contributions infeasible for me.
> >
> > The github workflow makes it impossible to open an issue or to
> > comment on an existing issue or to provide feedback about a patch
> > without being a github customer, and becoming a github customer
> > is not an option for me (and quite a number of other open-source
> > developers) as the github TOS contain clauses that I (and other
> > people) consider completely unacceptable.
> >
> > Besides the aforementioned points I am generally concerned about
> > the closed nature of the github issue- and pull-request system.
> > While it is of course easily possible to move a git repo from
> > github to somewhere else, it is as far as I know (please correct
> > me if I should me misinformed here) not possible to export the
> > comments and discussions in issues and pull requests in any
> > meaningful way to some other hosting platform, which creates a
> > strong vendor-lock-in once a project starts using the github
> > issue and pull-request facilities.  With the traditional
> > mailinglist-based workflow on the other hand, moving everything
> > to another hosting platform is trivial, so vendor-lock-in
> > is not a problem there.
> >
> > Another problem that I see in the github workflow is that it
> > requires being online all the time while the mailinglist-based
> > workflow makes it easy to read and comment on patches while being
> > offline.  I am sure that many people will now think "everybody is
> > online all day nowadays", but that's not true everywhere.  I for
> > example travel a lot by train and use the time on the train for
> > catching up with current developments and for reviewing things.
> > Where I live, for most practical purposes being on the train
> > effectively means being offline as far as modern web applications
> > are concerned.  Availability of mobile internet access is spotty
> > at best, and if one has internet connectivity inside the train at
> > all, it is often so slow that using it for interactive work on a
> > web interface is not feasible.  Receiving, writing and sending
> > emails on the other hand works without problems even with spotty
> > and slow internet connectivity.
>
> Just to clarify, my question was whether we should add a new workflow.
> I don't think there is any interest in removing the mailing-list flow.
>
> Re your comments about the TOS - what specifically causes problems? Re
> exporting comment I have been wondering that...is there no API for it?
> Finally, do your comments apply to gerrit and gitlab as well?

We are using gerrit and works for us in multi-project repo very well
as in single project.
We have verification build connected to jenkins. I prefer gerrit over
github and gitlab becuse
I still don't found a way to review commit message itseld on the other
system. Gerrit store in git
review and is moving forward even on comment using email. For the last
I tried some version ago.

Michael

>
> Regards,
> Simon



-- 
Michael Nazzareno Trimarchi
Amarula Solutions BV
COO Co-Founder
Cruquiuskade 47 Amsterdam 1018 AM NL
T. +31(0)851119172
M. +39(0)3479132170
[`as] https://www.amarulasolutions.com