[PATCH v1 3/3] arm: lpc32xx: add EA LPC3250 DevKitv2 board support

2021-06-06 Thread Trevor Woerner
Add basic support for running U-Boot on the Embedded Artists LPC3250
Developer's Kit v2 board by launching U-Boot from the board's s1l loader
(which comes pre-installed on the board).

Signed-off-by: Trevor Woerner 
---

 arch/arm/dts/Makefile |   2 +
 arch/arm/dts/lpc3250-ea3250-u-boot.dtsi   |  15 ++
 arch/arm/mach-lpc32xx/Kconfig |   4 +
 board/ea/ea-lpc3250devkitv2/Kconfig   |  15 ++
 board/ea/ea-lpc3250devkitv2/MAINTAINERS   |   9 ++
 board/ea/ea-lpc3250devkitv2/Makefile  |   4 +
 board/ea/ea-lpc3250devkitv2/README.rst| 132 ++
 .../ea-lpc3250devkitv2/ea-lpc3250devkitv2.c   |  37 +
 configs/ea-lpc3250devkitv2_defconfig  |  23 +++
 include/configs/ea-lpc3250devkitv2.h  |  37 +
 10 files changed, 278 insertions(+)
 create mode 100644 arch/arm/dts/lpc3250-ea3250-u-boot.dtsi
 create mode 100644 board/ea/ea-lpc3250devkitv2/Kconfig
 create mode 100644 board/ea/ea-lpc3250devkitv2/MAINTAINERS
 create mode 100644 board/ea/ea-lpc3250devkitv2/Makefile
 create mode 100644 board/ea/ea-lpc3250devkitv2/README.rst
 create mode 100644 board/ea/ea-lpc3250devkitv2/ea-lpc3250devkitv2.c
 create mode 100644 configs/ea-lpc3250devkitv2_defconfig
 create mode 100644 include/configs/ea-lpc3250devkitv2.h

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 096068261d..134108f14e 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1103,6 +1103,8 @@ dtb-$(CONFIG_TARGET_PRESIDIO_ASIC) += 
ca-presidio-engboard.dtb
 
 dtb-$(CONFIG_TARGET_IMX8MM_CL_IOT_GATE) += imx8mm-cl-iot-gate.dtb
 
+dtb-$(CONFIG_TARGET_EA_LPC3250DEVKITV2) += lpc3250-ea3250.dtb
+
 targets += $(dtb-y)
 
 # Add any required device tree compiler flags here
diff --git a/arch/arm/dts/lpc3250-ea3250-u-boot.dtsi 
b/arch/arm/dts/lpc3250-ea3250-u-boot.dtsi
new file mode 100644
index 00..0c82e512c6
--- /dev/null
+++ b/arch/arm/dts/lpc3250-ea3250-u-boot.dtsi
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021 Trevor Woerner 
+ */
+
+/{
+   model = "Embedded Artists LPC3250 DevKit v2 board based on the NXP 
LPC3250 SoC";
+   chosen {
+   stdout-path = 
+   };
+};
+
+ {
+   compatible = "nxp,lpc3220-uart", "ns16550a";
+};
diff --git a/arch/arm/mach-lpc32xx/Kconfig b/arch/arm/mach-lpc32xx/Kconfig
index 986ad738ac..185bda41c2 100644
--- a/arch/arm/mach-lpc32xx/Kconfig
+++ b/arch/arm/mach-lpc32xx/Kconfig
@@ -12,9 +12,13 @@ config TARGET_DEVKIT3250
 config TARGET_WORK_92105
bool "Work Microwave Work_92105"
 
+config TARGET_EA_LPC3250DEVKITV2
+   bool "Embedded Artists LPC3250 Developer's Kit v2"
+
 endchoice
 
 source "board/timll/devkit3250/Kconfig"
 source "board/work-microwave/work_92105/Kconfig"
+source "board/ea/ea-lpc3250devkitv2/Kconfig"
 
 endif
diff --git a/board/ea/ea-lpc3250devkitv2/Kconfig 
b/board/ea/ea-lpc3250devkitv2/Kconfig
new file mode 100644
index 00..368ce027e6
--- /dev/null
+++ b/board/ea/ea-lpc3250devkitv2/Kconfig
@@ -0,0 +1,15 @@
+if TARGET_EA_LPC3250DEVKITV2
+
+config SYS_BOARD
+   default "ea-lpc3250devkitv2"
+
+config SYS_VENDOR
+   default "ea"
+
+config SYS_SOC
+   default "lpc32xx"
+
+config SYS_CONFIG_NAME
+   default "ea-lpc3250devkitv2"
+
+endif
diff --git a/board/ea/ea-lpc3250devkitv2/MAINTAINERS 
b/board/ea/ea-lpc3250devkitv2/MAINTAINERS
new file mode 100644
index 00..b4b9362f5b
--- /dev/null
+++ b/board/ea/ea-lpc3250devkitv2/MAINTAINERS
@@ -0,0 +1,9 @@
+EMBEDDED ARTISTS LPC3250 DEVKIT v2
+M: Trevor Woerner 
+S: Maintained
+F: board/ea/ea-lpc3250devkitv2
+F: include/configs/ea-lpc3250devkitv2.h
+F: configs/ea-lpc3250devkitv2_defconfig
+F: arch/arm/dts/lpc32xx.dtsi
+F: arch/arm/dts/lpc3250-ea3250.dts
+F: arch/arm/dts/lpc3250-ea3250-u-boot.dtsi
diff --git a/board/ea/ea-lpc3250devkitv2/Makefile 
b/board/ea/ea-lpc3250devkitv2/Makefile
new file mode 100644
index 00..a4a40b6d4f
--- /dev/null
+++ b/board/ea/ea-lpc3250devkitv2/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2021  Trevor Woerner 
+
+obj-y += ea-lpc3250devkitv2.o
diff --git a/board/ea/ea-lpc3250devkitv2/README.rst 
b/board/ea/ea-lpc3250devkitv2/README.rst
new file mode 100644
index 00..56b5d0dbb1
--- /dev/null
+++ b/board/ea/ea-lpc3250devkitv2/README.rst
@@ -0,0 +1,132 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+ToC:
+- Introduction
+- Booting
+- Debugging
+
+
+Introduction
+
+The Embedded Artists LPC3250 Developer's Kit v2 features the LPC3250 SoC
+which is based on the ARM926EJ-S CPU. The kit features a base board and
+a removable OEM board which features the SoC. Details, schematics, and
+documentation are available from the Embedded Artists product website:
+
+   https://www.embeddedartists.com/products/lpc3250-developers-kit-v2/
+
+The base board includes::
+- 200 pos, 0.6mm pitch SODIMM connector for OEM Board
+- LCD expansion connector with 

[PATCH v1 2/3] lpc32xx: import device tree from Linux

2021-06-06 Thread Trevor Woerner
Import the dtsi, dts, and clock binding files for the lpc32xx ea3250 board
directly and unmodified from the latest Linux kernel.

Signed-off-by: Trevor Woerner 
---

 arch/arm/dts/lpc3250-ea3250.dts   | 273 
 arch/arm/dts/lpc32xx.dtsi | 508 ++
 include/dt-bindings/clock/lpc32xx-clock.h |  58 +++
 3 files changed, 839 insertions(+)
 create mode 100644 arch/arm/dts/lpc3250-ea3250.dts
 create mode 100644 arch/arm/dts/lpc32xx.dtsi
 create mode 100644 include/dt-bindings/clock/lpc32xx-clock.h

diff --git a/arch/arm/dts/lpc3250-ea3250.dts b/arch/arm/dts/lpc3250-ea3250.dts
new file mode 100644
index 00..63c6f17bb7
--- /dev/null
+++ b/arch/arm/dts/lpc3250-ea3250.dts
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Embedded Artists LPC3250 board
+ *
+ * Copyright 2012 Roland Stigge 
+ */
+
+/dts-v1/;
+#include "lpc32xx.dtsi"
+
+/ {
+   model = "Embedded Artists LPC3250 board based on NXP LPC3250";
+   compatible = "ea,ea3250", "nxp,lpc3250";
+
+   memory@8000 {
+   device_type = "memory";
+   reg = <0x8000 0x400>;
+   };
+
+   gpio-keys {
+   compatible = "gpio-keys";
+   autorepeat;
+
+   button {
+   label = "Interrupt Key";
+   linux,code = <103>;
+   gpios = < 4 1 0>; /* GPI_P3 1 */
+   };
+
+   key1 {
+   label = "KEY1";
+   linux,code = <1>;
+   gpios = < 0 0>;
+   };
+
+   key2 {
+   label = "KEY2";
+   linux,code = <2>;
+   gpios = < 1 0>;
+   };
+
+   key3 {
+   label = "KEY3";
+   linux,code = <3>;
+   gpios = < 2 0>;
+   };
+
+   key4 {
+   label = "KEY4";
+   linux,code = <4>;
+   gpios = < 3 0>;
+   };
+
+   joy0 {
+   label = "Joystick Key 0";
+   linux,code = <10>;
+   gpios = < 2 0 0>; /* P2.0 */
+   };
+
+   joy1 {
+   label = "Joystick Key 1";
+   linux,code = <11>;
+   gpios = < 2 1 0>; /* P2.1 */
+   };
+
+   joy2 {
+   label = "Joystick Key 2";
+   linux,code = <12>;
+   gpios = < 2 2 0>; /* P2.2 */
+   };
+
+   joy3 {
+   label = "Joystick Key 3";
+   linux,code = <13>;
+   gpios = < 2 3 0>; /* P2.3 */
+   };
+
+   joy4 {
+   label = "Joystick Key 4";
+   linux,code = <14>;
+   gpios = < 2 4 0>; /* P2.4 */
+   };
+   };
+
+   leds {
+   compatible = "gpio-leds";
+
+   /* LEDs on OEM Board */
+
+   led1 {
+   gpios = < 5 14 1>; /* GPO_P3 14, GPIO 93, active 
low */
+   linux,default-trigger = "timer";
+   default-state = "off";
+   };
+
+   led2 {
+   gpios = < 2 10 1>; /* P2.10, active low */
+   default-state = "off";
+   };
+
+   led3 {
+   gpios = < 2 11 1>; /* P2.11, active low */
+   default-state = "off";
+   };
+
+   led4 {
+   gpios = < 2 12 1>; /* P2.12, active low */
+   default-state = "off";
+   };
+
+   /* LEDs on Base Board */
+
+   lede1 {
+   gpios = < 8 0>;
+   default-state = "off";
+   };
+   lede2 {
+   gpios = < 9 0>;
+   default-state = "off";
+   };
+   lede3 {
+   gpios = < 10 0>;
+   default-state = "off";
+   };
+   lede4 {
+   gpios = < 11 0>;
+   default-state = "off";
+   };
+   lede5 {
+   gpios = < 12 0>;
+   default-state = "off";
+   };
+   lede6 {
+   gpios = < 13 0>;
+   default-state = "off";
+   };
+   lede7 {
+   gpios = < 14 0>;
+   default-state = "off";
+   };
+   lede8 {
+   gpios = < 15 0>;
+   default-state = "off";
+   };
+   };
+};
+
+/* 

[PATCH v1 1/3] lpc32xx: Kconfig: switch to CONFIG_CONS_INDEX

2021-06-06 Thread Trevor Woerner
There's nothing special or unique to the lpc32xx that requires its own config
parameter for specifying the console uart index. Therefore instead of using
the lpc32xx-specific CONFIG_SYS_LPC32XX_UART include parameter, use the
already-available CONFIG_CONS_INDEX from Kconfig.

Signed-off-by: Trevor Woerner 
---

 arch/arm/include/asm/arch-lpc32xx/config.h   | 4 ++--
 arch/arm/mach-lpc32xx/devices.c  | 3 +--
 board/timll/devkit3250/devkit3250.c  | 2 +-
 board/timll/devkit3250/devkit3250_spl.c  | 2 +-
 board/work-microwave/work_92105/work_92105.c | 2 +-
 board/work-microwave/work_92105/work_92105_spl.c | 2 +-
 configs/devkit3250_defconfig | 2 ++
 configs/work_92105_defconfig | 2 ++
 include/configs/devkit3250.h | 5 -
 include/configs/work_92105.h | 5 -
 scripts/config_whitelist.txt | 1 -
 11 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/arch-lpc32xx/config.h 
b/arch/arm/include/asm/arch-lpc32xx/config.h
index 0836091af2..45e46f9946 100644
--- a/arch/arm/include/asm/arch-lpc32xx/config.h
+++ b/arch/arm/include/asm/arch-lpc32xx/config.h
@@ -12,8 +12,8 @@
 /* Basic CPU architecture */
 
 /* UART configuration */
-#if(CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
-   (CONFIG_SYS_LPC32XX_UART == 7)
+#if(CONFIG_CONS_INDEX == 1) || (CONFIG_CONS_INDEX == 2) || \
+   (CONFIG_CONS_INDEX == 7)
 #if !defined(CONFIG_LPC32XX_HSUART)
 #define CONFIG_LPC32XX_HSUART
 #endif
diff --git a/arch/arm/mach-lpc32xx/devices.c b/arch/arm/mach-lpc32xx/devices.c
index e1e2e0d094..0a4fef295a 100644
--- a/arch/arm/mach-lpc32xx/devices.c
+++ b/arch/arm/mach-lpc32xx/devices.c
@@ -23,8 +23,7 @@ void lpc32xx_uart_init(unsigned int uart_id)
return;
 
/* Disable loopback mode, if it is set by S1L bootloader */
-   clrbits_le32(>loop,
-UART_LOOPBACK(CONFIG_SYS_LPC32XX_UART));
+   clrbits_le32(>loop, UART_LOOPBACK(uart_id));
 
if (uart_id < 3 || uart_id > 6)
return;
diff --git a/board/timll/devkit3250/devkit3250.c 
b/board/timll/devkit3250/devkit3250.c
index 3c744b943f..9d4ffb0f97 100644
--- a/board/timll/devkit3250/devkit3250.c
+++ b/board/timll/devkit3250/devkit3250.c
@@ -38,7 +38,7 @@ void reset_periph(void)
 
 int board_early_init_f(void)
 {
-   lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
+   lpc32xx_uart_init(CONFIG_CONS_INDEX);
lpc32xx_i2c_init(1);
lpc32xx_i2c_init(2);
lpc32xx_ssp_init();
diff --git a/board/timll/devkit3250/devkit3250_spl.c 
b/board/timll/devkit3250/devkit3250_spl.c
index 47af78ae0b..12e8ae9c39 100644
--- a/board/timll/devkit3250/devkit3250_spl.c
+++ b/board/timll/devkit3250/devkit3250_spl.c
@@ -49,7 +49,7 @@ void spl_board_init(void)
/* First of all silence buzzer controlled by GPO_20 */
writel((1 << 20), >p3_outp_clr);
 
-   lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
+   lpc32xx_uart_init(CONFIG_CONS_INDEX);
preloader_console_init();
 
ddr_init(_64mb);
diff --git a/board/work-microwave/work_92105/work_92105.c 
b/board/work-microwave/work_92105/work_92105.c
index bdcecff730..5d12f84cfe 100644
--- a/board/work-microwave/work_92105/work_92105.c
+++ b/board/work-microwave/work_92105/work_92105.c
@@ -37,7 +37,7 @@ void reset_periph(void)
 int board_early_init_f(void)
 {
/* initialize serial port for console */
-   lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
+   lpc32xx_uart_init(CONFIG_CONS_INDEX);
/* enable I2C, SSP, MAC, NAND */
lpc32xx_i2c_init(1); /* only I2C1 has devices, I2C2 has none */
lpc32xx_ssp_init();
diff --git a/board/work-microwave/work_92105/work_92105_spl.c 
b/board/work-microwave/work_92105/work_92105_spl.c
index a31553a2d2..d9401145f2 100644
--- a/board/work-microwave/work_92105/work_92105_spl.c
+++ b/board/work-microwave/work_92105/work_92105_spl.c
@@ -58,7 +58,7 @@ const struct emc_dram_settings dram_128mb = {
 void spl_board_init(void)
 {
/* initialize serial port for console */
-   lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
+   lpc32xx_uart_init(CONFIG_CONS_INDEX);
/* initialize console */
preloader_console_init();
/* init DDR and NAND to chainload U-Boot */
diff --git a/configs/devkit3250_defconfig b/configs/devkit3250_defconfig
index 93c048cee8..9ae70f7d46 100644
--- a/configs/devkit3250_defconfig
+++ b/configs/devkit3250_defconfig
@@ -51,6 +51,8 @@ CONFIG_PHYLIB=y
 CONFIG_PHY_ADDR_ENABLE=y
 CONFIG_PHY_ADDR=31
 CONFIG_PHY_SMSC=y
+CONFIG_SPECIFY_CONSOLE_INDEX=y
+CONFIG_CONS_INDEX=5
 CONFIG_SYS_NS16550=y
 CONFIG_SPI=y
 CONFIG_USB=y
diff --git a/configs/work_92105_defconfig b/configs/work_92105_defconfig
index c3f666dcfe..e9605adedd 100644
--- a/configs/work_92105_defconfig
+++ b/configs/work_92105_defconfig
@@ -48,5 +48,7 @@ CONFIG_MTD_RAW_NAND=y
 

[PATCH v1 0/3] lpc32xx updates

2021-06-06 Thread Trevor Woerner
This patch series starts with some lpc32xx-related cleanups, then adds
support for a new LPC32XX board: the Embedded Artists LPC3250 DevKit v2


Trevor Woerner (3):
  lpc32xx: Kconfig: switch to CONFIG_CONS_INDEX
  lpc32xx: import device tree from Linux
  arm: lpc32xx: add EA LPC3250 DevKitv2 board support

 arch/arm/dts/Makefile |   2 +
 arch/arm/dts/lpc3250-ea3250-u-boot.dtsi   |  15 +
 arch/arm/dts/lpc3250-ea3250.dts   | 273 ++
 arch/arm/dts/lpc32xx.dtsi | 508 ++
 arch/arm/include/asm/arch-lpc32xx/config.h|   4 +-
 arch/arm/mach-lpc32xx/Kconfig |   4 +
 arch/arm/mach-lpc32xx/devices.c   |   3 +-
 board/ea/ea-lpc3250devkitv2/Kconfig   |  15 +
 board/ea/ea-lpc3250devkitv2/MAINTAINERS   |   9 +
 board/ea/ea-lpc3250devkitv2/Makefile  |   4 +
 board/ea/ea-lpc3250devkitv2/README.rst| 132 +
 .../ea-lpc3250devkitv2/ea-lpc3250devkitv2.c   |  37 ++
 board/timll/devkit3250/devkit3250.c   |   2 +-
 board/timll/devkit3250/devkit3250_spl.c   |   2 +-
 board/work-microwave/work_92105/work_92105.c  |   2 +-
 .../work_92105/work_92105_spl.c   |   2 +-
 configs/devkit3250_defconfig  |   2 +
 configs/ea-lpc3250devkitv2_defconfig  |  23 +
 configs/work_92105_defconfig  |   2 +
 include/configs/devkit3250.h  |   5 -
 include/configs/ea-lpc3250devkitv2.h  |  37 ++
 include/configs/work_92105.h  |   5 -
 include/dt-bindings/clock/lpc32xx-clock.h |  58 ++
 scripts/config_whitelist.txt  |   1 -
 24 files changed, 1128 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm/dts/lpc3250-ea3250-u-boot.dtsi
 create mode 100644 arch/arm/dts/lpc3250-ea3250.dts
 create mode 100644 arch/arm/dts/lpc32xx.dtsi
 create mode 100644 board/ea/ea-lpc3250devkitv2/Kconfig
 create mode 100644 board/ea/ea-lpc3250devkitv2/MAINTAINERS
 create mode 100644 board/ea/ea-lpc3250devkitv2/Makefile
 create mode 100644 board/ea/ea-lpc3250devkitv2/README.rst
 create mode 100644 board/ea/ea-lpc3250devkitv2/ea-lpc3250devkitv2.c
 create mode 100644 configs/ea-lpc3250devkitv2_defconfig
 create mode 100644 include/configs/ea-lpc3250devkitv2.h
 create mode 100644 include/dt-bindings/clock/lpc32xx-clock.h

-- 
2.30.0.rc0



Re: [PATCH v3 00/10] AM642-EVM: Add USB support

2021-06-06 Thread Praneeth Bajjuri




On 6/4/21 11:53 AM, Praneeth Bajjuri wrote:

Aswath, Suman, Dave, Gowtham, Vignesh

On 6/4/21 11:49 AM, Aswath Govindraju wrote:

Hi Praneeth,

On 04/06/21 10:18 pm, Praneeth Bajjuri wrote:



On 6/4/21 11:30 AM, Aswath Govindraju wrote:

The following series of patches add support for the following
- Kconfig symbol for giving the load address for ATF
- USB Mass storage boot mode in AM642-EVM
- DFU boot mode in AM642-EVM
- Host and peripheral modes for AM642-EVM in U-Boot
- Set the USB PHY core voltage to 0.85V

changes since v2,
- Increased the max size of ATF in patch 9


Please confirm if this is tested on ATF 2.5 too.



Sorry, for not mentioning it.

Yes, this series was tested using ATF 2.5.


Ok , pulling to ti-u-boot.

Aswath:
1. Please send the updated series to upstream u-boot as well.
2. post the kernel dts change to mainline as well.

Vignesh,
Assuming you are pulling the KIG patch too.

Suman/Dave/Gowtham,
Assuming you are pulling the kernel patch too.


Please ignore this mail. was reviewing product baseline for same feature.







Thanks,
Aswath




- Added reviewed-by from Suman Anna in patch 1
- Reworded the subject of patch 8

changes since v1,
- Corrected the default load address of ATF to
    0x7000

Aswath Govindraju (10):
    tools: k3_fit_atf: Add support for providing ATF load address 
using a

  Kconfig symbol
    arm: mach-k3: am642_init: Add support for USB boot mode
    arm: mach-k3: am642_init: Do USB fixups to facilitate host and 
device

  boot modes
    board: ti: am64x: Set the core voltage of USB PHY to 0.85V
    arm: dts: k3-am64-main: Add USB DT nodes
    arm: dts: k3-am642-*-evm: Add USB support
    arm: dts: k3-am642-evm-u-boot: Add U-Boot tags and fix the 
dr_mode to

  peripheral for USB subsystem
    configs: am64x_evm_*_defconfig: Rearrange the components in SRAM to
  satisfy the limitations for USB DFU boot mode
    arm: dts: k3-am64-main: Update the location of ATF in SRAM and
  increase its max size
    configs: am64: Enable configs to support USB host and device modes

   arch/arm/dts/k3-am64-main.dtsi    | 32 -
   arch/arm/dts/k3-am642-evm-u-boot.dtsi | 13 ++
   arch/arm/dts/k3-am642-evm.dts | 18 
   arch/arm/dts/k3-am642-r5-evm.dts  | 18 
   arch/arm/mach-k3/Kconfig  |  7 +++
   arch/arm/mach-k3/am642_init.c | 46 
++-

   arch/arm/mach-k3/config.mk    |  1 +
   arch/arm/mach-k3/include/mach/am64_hardware.h | 11 +++--
   arch/arm/mach-k3/include/mach/am64_spl.h  |  6 ++-
   board/ti/am64x/evm.c  | 14 ++
   configs/am64x_evm_a53_defconfig   | 40 
   configs/am64x_evm_r5_defconfig    | 38 +--
   include/configs/am64x_evm.h   | 15 +-
   tools/k3_fit_atf.sh   |  9 ++--
   14 files changed, 251 insertions(+), 17 deletions(-)





[PATCH 2/2] configs: rockchip: rk3399: Khadas Edge add USB OHCI

2021-06-06 Thread Artem Lapkin
Problem: USB2.0 port can recognize any USB1.1 devices (like usb keyboard)
Add missed USB OHCI configuration

USB device tree:
  1  Hub (480 Mb/s, 0mA)
 u-boot EHCI Host Controller

  1  Hub (12 Mb/s, 0mA)
  |   U-Boot Root Hub
  |
  +-2  Human Interface (1.5 Mb/s, 100mA)
Dell KB216 Wired Keyboard

Signed-off-by: Artem Lapkin 
---
 configs/khadas-edge-captain-rk3399_defconfig | 2 ++
 configs/khadas-edge-rk3399_defconfig | 2 ++
 configs/khadas-edge-v-rk3399_defconfig   | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/configs/khadas-edge-captain-rk3399_defconfig 
b/configs/khadas-edge-captain-rk3399_defconfig
index 63074a4eed..ce6b492b3d 100644
--- a/configs/khadas-edge-captain-rk3399_defconfig
+++ b/configs/khadas-edge-captain-rk3399_defconfig
@@ -50,6 +50,8 @@ CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_GENERIC=y
 CONFIG_USB_HOST_ETHER=y
 CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_ASIX88179=y
diff --git a/configs/khadas-edge-rk3399_defconfig 
b/configs/khadas-edge-rk3399_defconfig
index cf5a6da384..e52963e86f 100644
--- a/configs/khadas-edge-rk3399_defconfig
+++ b/configs/khadas-edge-rk3399_defconfig
@@ -49,6 +49,8 @@ CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_GENERIC=y
 CONFIG_USB_HOST_ETHER=y
 CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_ASIX88179=y
diff --git a/configs/khadas-edge-v-rk3399_defconfig 
b/configs/khadas-edge-v-rk3399_defconfig
index 197a6f6677..5f61df85da 100644
--- a/configs/khadas-edge-v-rk3399_defconfig
+++ b/configs/khadas-edge-v-rk3399_defconfig
@@ -50,6 +50,8 @@ CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_GENERIC=y
 CONFIG_USB_HOST_ETHER=y
 CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_ASIX88179=y
-- 
2.25.1



[PATCH 1/2] ARM64: rockchip: evb_rk3399: add usb ohci definations

2021-06-06 Thread Artem Lapkin
Problem: USB2.0 port can recognize any USB1.1 devices (like usb keyboard)
Add missed USB OHCI configuration

USB device tree:
  1  Hub (480 Mb/s, 0mA)
 u-boot EHCI Host Controller

  1  Hub (12 Mb/s, 0mA)
  |   U-Boot Root Hub
  |
  +-2  Human Interface (1.5 Mb/s, 100mA)
Dell KB216 Wired Keyboard

Signed-off-by: Artem Lapkin 
---
 include/configs/evb_rk3399.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/evb_rk3399.h b/include/configs/evb_rk3399.h
index b7e850370b..492b7b4df1 100644
--- a/include/configs/evb_rk3399.h
+++ b/include/configs/evb_rk3399.h
@@ -15,4 +15,7 @@
 
 #define SDRAM_BANK_SIZE(2UL << 30)
 
+#define CONFIG_USB_OHCI_NEW
+#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 2
+
 #endif
-- 
2.25.1



[PATCH 0/2] ARM64: rockchip: evb_rk3399: Khadas Edge add USB OHCI

2021-06-06 Thread Artem Lapkin
Problem: USB2.0 port can recognize any USB1.1 devices (like usb keyboard)
Add missed USB OHCI configuration

Artem Lapkin (2):
  ARM64: rockchip: evb_rk3399: add usb ohci definations
  configs: rockchip: rk3399: Khadas Edge add USB OHCI

 configs/khadas-edge-captain-rk3399_defconfig | 2 ++
 configs/khadas-edge-rk3399_defconfig | 2 ++
 configs/khadas-edge-v-rk3399_defconfig   | 2 ++
 include/configs/evb_rk3399.h | 3 +++
 4 files changed, 9 insertions(+)

-- 
2.25.1



Pull request: u-boot-rockchip-20210606

2021-06-06 Thread Kever Yang
Hi Tom,

Please pull the rockchip updates/fixes:
- Maintainer mail address update

DENX ci:
https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/7676

Thanks,
- Kever

The following changes since commit d8729a114e1e98806cffb87d2dca895f99a0170a:

  Merge https://source.denx.de/u-boot/custodians/u-boot-riscv (2021-05-31 
10:19:14 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-rockchip.git 
tags/u-boot-rockchip-20210606

for you to fetch changes up to 87f9c08d845b65e8dd301d64ae2692d1e694d8ab:

  MAINTAINERS: Update maintainer's mail address (2021-06-01 19:57:02 +0800)


Kever Yang (1):
  MAINTAINERS: Update maintainer's mail address

 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)




Re: [PATCH] riscv: ae350: doc: Remove CONFIG_SKIP_LOWLEVEL_INIT

2021-06-06 Thread Rick Chen
> From: Bin Meng 
> Sent: Saturday, June 05, 2021 7:01 AM
> To: Rick Jian-Zhi Chen(陳建志) ; Leo Yu-Chi Liang(梁育齊) 
> ; u-boot@lists.denx.de
> Cc: Bin Meng 
> Subject: [PATCH] riscv: ae350: doc: Remove CONFIG_SKIP_LOWLEVEL_INIT
>
> The doc says CONFIG_SKIP_LOWLEVEL_INIT is in ax25-ae350.h, while actually it 
> is not. Remove it.
>
> Signed-off-by: Bin Meng 
> ---
>
>  doc/board/AndesTech/ax25-ae350.rst | 19 ---
>  1 file changed, 4 insertions(+), 15 deletions(-)

Reviewed-by: Rick Chen 


[PATCH] Revert "mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to control card clock output"

2021-06-06 Thread Fabio Estevam
This reverts commit 63756575b42b8b4fb3f59cbbf0cedf03331bc2d2.

Since this commit a imx6qdl-pico board boots extremely slowly
in both SPL as well as U-Boot proper.

Fix this revert for now to avoid such regression.

Signed-off-by: Fabio Estevam 
---
 drivers/mmc/fsl_esdhc_imx.c | 29 -
 include/fsl_esdhc_imx.h |  2 --
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index a4675838e58a..b49d5ede2711 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -653,10 +653,7 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct 
mmc *mmc, uint clock)
clk = (pre_div << 8) | (div << 4);
 
 #ifdef CONFIG_FSL_USDHC
-   esdhc_clrbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
-   ret = readx_poll_timeout(esdhc_read32, >prsstat, tmp, tmp & 
PRSSTAT_SDOFF, 100);
-   if (ret)
-   pr_warn("fsl_esdhc_imx: Internal clock never gate off.\n");
+   esdhc_clrbits32(>vendorspec, VENDORSPEC_CKEN);
 #else
esdhc_clrbits32(>sysctl, SYSCTL_CKEN);
 #endif
@@ -668,7 +665,7 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct 
mmc *mmc, uint clock)
pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
 
 #ifdef CONFIG_FSL_USDHC
-   esdhc_setbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+   esdhc_setbits32(>vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
 #else
esdhc_setbits32(>sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 #endif
@@ -723,14 +720,8 @@ static void esdhc_set_strobe_dll(struct mmc *mmc)
struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
struct fsl_esdhc *regs = priv->esdhc_regs;
u32 val;
-   u32 tmp;
-   int ret;
 
if (priv->clock > ESDHC_STROBE_DLL_CLK_FREQ) {
-   esdhc_clrbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
-   ret = readx_poll_timeout(esdhc_read32, >prsstat, tmp, tmp 
& PRSSTAT_SDOFF, 100);
-   if (ret)
-   pr_warn("fsl_esdhc_imx: Internal clock never gate 
off.\n");
esdhc_write32(>strobe_dllctrl, 
ESDHC_STROBE_DLL_CTRL_RESET);
 
/*
@@ -748,7 +739,6 @@ static void esdhc_set_strobe_dll(struct mmc *mmc)
pr_warn("HS400 strobe DLL status REF not lock!\n");
if (!(val & ESDHC_STROBE_DLL_STS_SLV_LOCK))
pr_warn("HS400 strobe DLL status SLV not lock!\n");
-   esdhc_setbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
}
 }
 
@@ -980,18 +970,14 @@ static int esdhc_set_ios_common(struct fsl_esdhc_priv 
*priv, struct mmc *mmc)
 #ifdef MMC_SUPPORTS_TUNING
if (mmc->clk_disable) {
 #ifdef CONFIG_FSL_USDHC
-   u32 tmp;
-
-   esdhc_clrbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
-   ret = readx_poll_timeout(esdhc_read32, >prsstat, tmp, tmp 
& PRSSTAT_SDOFF, 100);
-   if (ret)
-   pr_warn("fsl_esdhc_imx: Internal clock never gate 
off.\n");
+   esdhc_clrbits32(>vendorspec, VENDORSPEC_CKEN);
 #else
esdhc_clrbits32(>sysctl, SYSCTL_CKEN);
 #endif
} else {
 #ifdef CONFIG_FSL_USDHC
-   esdhc_setbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+   esdhc_setbits32(>vendorspec, VENDORSPEC_PEREN |
+   VENDORSPEC_CKEN);
 #else
esdhc_setbits32(>sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 #endif
@@ -1067,7 +1053,7 @@ static int esdhc_init_common(struct fsl_esdhc_priv *priv, 
struct mmc *mmc)
 #ifndef CONFIG_FSL_USDHC
esdhc_setbits32(>sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
 #else
-   esdhc_setbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+   esdhc_setbits32(>vendorspec, VENDORSPEC_HCKEN | VENDORSPEC_IPGEN);
 #endif
 
/* Set the initial clock speed */
@@ -1205,7 +1191,8 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
esdhc_write32(>autoc12err, 0);
esdhc_write32(>clktunectrlstatus, 0);
 #else
-   esdhc_setbits32(>vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+   esdhc_setbits32(>vendorspec, VENDORSPEC_PEREN |
+   VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
 #endif
 
if (priv->vs18_enable)
diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
index b0920344641c..45ed635a77bf 100644
--- a/include/fsl_esdhc_imx.h
+++ b/include/fsl_esdhc_imx.h
@@ -39,7 +39,6 @@
 #define VENDORSPEC_HCKEN   0x1000
 #define VENDORSPEC_IPGEN   0x0800
 #define VENDORSPEC_INIT0x20007809
-#define VENDORSPEC_FRC_SDCLK_ON 0x0100
 
 #define IRQSTAT0x0002e030
 #define IRQSTAT_DMAE   (0x1000)
@@ -97,7 +96,6 @@
 #define PRSSTAT_CINS   (0x0001)
 #define PRSSTAT_BREN   (0x0800)
 #define PRSSTAT_BWEN   (0x0400)
-#define PRSSTAT_SDOFF  (0x0080)
 #define PRSSTAT_SDSTB  

Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Heinrich Schuchardt
Am 6. Juni 2021 20:07:31 MESZ schrieb Sean Anderson :
>On 6/6/21 1:57 PM, Heinrich Schuchardt wrote:
>> On 6/6/21 7:52 PM, Sean Anderson wrote:
>>> On 6/6/21 1:28 PM, Heinrich Schuchardt wrote:
 On 6/6/21 6:44 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Mon, 22 Mar 2021 at 18:56, Simon Glass 
>wrote:
>>
>> Hi Heinrich,
>>
>> On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt
>>  wrote:
>>>
>>> On 22.03.21 06:21, Simon Glass wrote:
 At present if sandbox crashes it prints a message and tries to
 exit. But
 with the recently introduced signal handler, it often seems to
>get
 stuck
 in a loop until the stack overflows:

 Segmentation violation

 Segmentation violation

 Segmentation violation

 Segmentation violation

 Segmentation violation

 Segmentation violation

 Segmentation violation
 ...
>>>
>>> Hello Simon,
>>>
>>> do you have a reproducible example? I never have seen this.
>>
>> https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433
>>
>> You need to run that commit with pytest though...it does not
>happen
>> when run directly.
>>
>> BTW this sems to expose some rather nasty bug in dlmalloc or how
>it is
>> used. I notice that as soon as the first test is run, the 'top'
>value
>> in dlmalloc is outside the range of the malloc pool, which seems
>> wrong. I wonder if there is something broken with how
>> dm_test_pre_run() and dm_test_post_run() work.
>>
>>>
>>> Corrupting gd could cause an endless recursive loop, as these
>lines
>>> follow printing the observed string:
>>>
>>>  printf("pc = 0x%lx, ", pc);
>>>  printf("pc_reloc =0x%lx\n\n", pc - gd->reloc_off);
>>
>> Yes I suspect printf() is dead.
>>
>>>
>>> If we remove SA_NODEFER from the signal mask in
>arch/sandbox/cpu/os.c,
>>> recursion cannot occur anymore. If a segmentation violation
>occurs
>>> inside the handler it will be delegated to the default handler.
>>>
>>> Furthermore we could consider removing the signal handler at the
>start
>>> of os_signal_action().
>>
>> The issue is that if you get a segfault you really don't know if
>you
>> can continue and do anything else.
>>
>> What is the goal with the signal handler? I don't think the user
>can
>> do anything about it.

 Hello Simon,

 the signal handler prints out the crash location and this makes
 analyzing problems much easier. It proved valuable to me several
>times.
>>>
>>> Can't you just rerun with gdb?
>> 
>> This would require that the problem is easily reproducible which may
>not
>> be the case.
>
>Hm, perhaps you could keep track of how many times we've tried to catch
>a signal, and bail if this is the second time around. E.g. something
>like
>

Removing SA_NODEFER from the signal mask will let the OS kick in if an 
exception occurs in a signal handler.

No counter is needed.

Best regards

Heinrich

>static void os_signal_handler(int sig, siginfo_t *info, void *con)
>{
>   /* other variables */
>   static int level = 0;
>
>   switch (level++) {
>   case 0:
>   break;
>   case 1:
>   sandbox_exit();
>   default:
>   os_exit(0);
>   }
>
>   /* rest of the handler */
>}
>
>--Sean
>
>> 
>> Best regards
>> 
>> Heinrich
>> 
>>>

>
> I keep hitting this problem during development with sandbox, so I
> think I need to apply this patch.
>
> Does anything need to be updated in the tests?
>
> Regards,
> Simon
>

 Did you try removing SA_NODEFER as proposed?

 Best regards

 Heinrich
>>>
>>>
>> 



Re: [PATCH] env: Leave invalid env for nowhere location

2021-06-06 Thread Marek Vasut

On 6/3/21 6:15 PM, Kunihiko Hayashi wrote:

Hi Marek,


Hi,


Sorry for rate reply.


No worries, same here.


On 2021/05/25 16:35, Marek Vasut wrote:

On 5/12/21 4:09 PM, Kunihiko Hayashi wrote:
When CONFIG_ENV_IS_NOWHERE is enabled, env_nowhere_init() sets 
ENV_INVALID

to gd->env_valid, and sets default_environment before relocation to
gd->env_addr. After that, env_init() switches gd->env_valid to ENV_VALID
by the previous fix.

If gd->env_valid is ENV_INVALID, env_get_char() returns relocated
default_environment, however, env_get_char() returns gd->env_addr before
relocation since gd->env_valid is ENV_VALID, and access to gd->env_addr
will cause a fault.

This leaves gd->env_valid as ENV_INVALID for "nowhere" location.


So do I understand this correctly that _after_ relocation, env_init() is
called and env_init() does not update gd->env_addr to the relocated one?


In my understandings, env_init() belongs to init_sequence_f[]
and env_init() is called before relocation.


You're right.

So the env update after relocation should then be done in env_relocate().


I would expect that after relocation, if all you have is env_nowhere
driver, the env_nowhere_init() is called again from the first for() loop
of env_init() [1], which would set gd->env_valid to ENV_INVALID [1], and
that for() loop would exit with ret = -ENOENT [2], so then the last part
of env_init() would check for ret == -ENOENT and update gd->env_addr to
relocated default_environment [3].

324  int env_init(void)
325  {
326    struct env_driver *drv;
327    int ret = -ENOENT;
328    int prio;
329
330    for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); 
prio++) {

   /* Part [1] */
331  if (!drv->init || !(ret = drv->init()))
332    env_set_inited(drv->location);
333  if (ret == -ENOENT)
334    env_set_inited(drv->location);
335
336  debug("%s: Environment %s init done (ret=%d)\n", __func__,
337    drv->name, ret);
338
   /* Part [2] */
339  if (gd->env_valid == ENV_INVALID)
340    ret = -ENOENT;
341    }
342
343    if (!prio)
344  return -ENODEV;
345
 /* Part [3] */
346    if (ret == -ENOENT) {
   /* This should be relocated default_environment address */
347  gd->env_addr = (ulong)_environment[0];
348  gd->env_valid = ENV_VALID;
349
350  return 0;
351    }
352
353    return ret;
354  }

Or am I missing something obvious ?


These are called before relocation, and update gd->env_addr to 
non-relocated

default_environment by [3].

After that, gd->env_addr is relocated in initr_reloc_global_data()
if CONFIG_SYS_RELOC_GD_ENV_ADDR is defined.

| #ifdef CONFIG_SYS_RELOC_GD_ENV_ADDR
| /*
|  * Relocate the early env_addr pointer unless we know it is not 
inside
|  * the binary. Some systems need this and for the rest, it doesn't 
hurt.

|  */
| gd->env_addr += gd->reloc_off;
| #endif



Shouldn't the post-relocation env update happen in env_relocate() ?


However, I misunderstood my situation.
gd->reloc_off doesn't have the proper value because CONFIG_SYS_TEXT_BASE
is zero due to CONFIG_POSITION_INDENENDENT=y.

gd->reloc_off is calculated with CONFIG_SYS_TEXT_BASE in setup_reloc().

| gd->reloc_off = gd->relocaddr - CONFIG_SYS_TEXT_BASE;

gd->env_addr is added with gd->reloc_off (== gd->relocaddr - 0),
as a result, gd->env_addr has wrong address.

In this case, I think the proper solution is to undefine
CONFIG_SYS_RELOC_GD_ENV_ADDR.

My patch isn't necessary no longer and your patch also works with
"nowhere".

OK


Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Sean Anderson

On 6/6/21 1:57 PM, Heinrich Schuchardt wrote:

On 6/6/21 7:52 PM, Sean Anderson wrote:

On 6/6/21 1:28 PM, Heinrich Schuchardt wrote:

On 6/6/21 6:44 PM, Simon Glass wrote:

Hi Heinrich,

On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:


Hi Heinrich,

On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt
 wrote:


On 22.03.21 06:21, Simon Glass wrote:

At present if sandbox crashes it prints a message and tries to
exit. But
with the recently introduced signal handler, it often seems to get
stuck
in a loop until the stack overflows:

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation
...


Hello Simon,

do you have a reproducible example? I never have seen this.


https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433

You need to run that commit with pytest though...it does not happen
when run directly.

BTW this sems to expose some rather nasty bug in dlmalloc or how it is
used. I notice that as soon as the first test is run, the 'top' value
in dlmalloc is outside the range of the malloc pool, which seems
wrong. I wonder if there is something broken with how
dm_test_pre_run() and dm_test_post_run() work.



Corrupting gd could cause an endless recursive loop, as these lines
follow printing the observed string:

 printf("pc = 0x%lx, ", pc);
 printf("pc_reloc =0x%lx\n\n", pc - gd->reloc_off);


Yes I suspect printf() is dead.



If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
recursion cannot occur anymore. If a segmentation violation occurs
inside the handler it will be delegated to the default handler.

Furthermore we could consider removing the signal handler at the start
of os_signal_action().


The issue is that if you get a segfault you really don't know if you
can continue and do anything else.

What is the goal with the signal handler? I don't think the user can
do anything about it.


Hello Simon,

the signal handler prints out the crash location and this makes
analyzing problems much easier. It proved valuable to me several times.


Can't you just rerun with gdb?


This would require that the problem is easily reproducible which may not
be the case.


Hm, perhaps you could keep track of how many times we've tried to catch a 
signal, and bail if this is the second time around. E.g. something like

static void os_signal_handler(int sig, siginfo_t *info, void *con)
{
/* other variables */
static int level = 0;

switch (level++) {
case 0:
break;
case 1:
sandbox_exit();
default:
os_exit(0);
}

/* rest of the handler */
}

--Sean



Best regards

Heinrich







I keep hitting this problem during development with sandbox, so I
think I need to apply this patch.

Does anything need to be updated in the tests?

Regards,
Simon



Did you try removing SA_NODEFER as proposed?

Best regards

Heinrich









Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Heinrich Schuchardt

On 6/6/21 7:52 PM, Sean Anderson wrote:

On 6/6/21 1:28 PM, Heinrich Schuchardt wrote:

On 6/6/21 6:44 PM, Simon Glass wrote:

Hi Heinrich,

On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:


Hi Heinrich,

On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt
 wrote:


On 22.03.21 06:21, Simon Glass wrote:

At present if sandbox crashes it prints a message and tries to
exit. But
with the recently introduced signal handler, it often seems to get
stuck
in a loop until the stack overflows:

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation
...


Hello Simon,

do you have a reproducible example? I never have seen this.


https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433

You need to run that commit with pytest though...it does not happen
when run directly.

BTW this sems to expose some rather nasty bug in dlmalloc or how it is
used. I notice that as soon as the first test is run, the 'top' value
in dlmalloc is outside the range of the malloc pool, which seems
wrong. I wonder if there is something broken with how
dm_test_pre_run() and dm_test_post_run() work.



Corrupting gd could cause an endless recursive loop, as these lines
follow printing the observed string:

 printf("pc = 0x%lx, ", pc);
 printf("pc_reloc =0x%lx\n\n", pc - gd->reloc_off);


Yes I suspect printf() is dead.



If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
recursion cannot occur anymore. If a segmentation violation occurs
inside the handler it will be delegated to the default handler.

Furthermore we could consider removing the signal handler at the start
of os_signal_action().


The issue is that if you get a segfault you really don't know if you
can continue and do anything else.

What is the goal with the signal handler? I don't think the user can
do anything about it.


Hello Simon,

the signal handler prints out the crash location and this makes
analyzing problems much easier. It proved valuable to me several times.


Can't you just rerun with gdb?


This would require that the problem is easily reproducible which may not
be the case.

Best regards

Heinrich







I keep hitting this problem during development with sandbox, so I
think I need to apply this patch.

Does anything need to be updated in the tests?

Regards,
Simon



Did you try removing SA_NODEFER as proposed?

Best regards

Heinrich







Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Sean Anderson

On 6/6/21 1:28 PM, Heinrich Schuchardt wrote:

On 6/6/21 6:44 PM, Simon Glass wrote:

Hi Heinrich,

On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:


Hi Heinrich,

On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt  wrote:


On 22.03.21 06:21, Simon Glass wrote:

At present if sandbox crashes it prints a message and tries to exit. But
with the recently introduced signal handler, it often seems to get stuck
in a loop until the stack overflows:

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation
...


Hello Simon,

do you have a reproducible example? I never have seen this.


https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433

You need to run that commit with pytest though...it does not happen
when run directly.

BTW this sems to expose some rather nasty bug in dlmalloc or how it is
used. I notice that as soon as the first test is run, the 'top' value
in dlmalloc is outside the range of the malloc pool, which seems
wrong. I wonder if there is something broken with how
dm_test_pre_run() and dm_test_post_run() work.



Corrupting gd could cause an endless recursive loop, as these lines
follow printing the observed string:

 printf("pc = 0x%lx, ", pc);
 printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);


Yes I suspect printf() is dead.



If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
recursion cannot occur anymore. If a segmentation violation occurs
inside the handler it will be delegated to the default handler.

Furthermore we could consider removing the signal handler at the start
of os_signal_action().


The issue is that if you get a segfault you really don't know if you
can continue and do anything else.

What is the goal with the signal handler? I don't think the user can
do anything about it.


Hello Simon,

the signal handler prints out the crash location and this makes
analyzing problems much easier. It proved valuable to me several times.


Can't you just rerun with gdb?

--Sean





I keep hitting this problem during development with sandbox, so I
think I need to apply this patch.

Does anything need to be updated in the tests?

Regards,
Simon



Did you try removing SA_NODEFER as proposed?

Best regards

Heinrich





Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Heinrich Schuchardt

On 6/6/21 7:37 PM, Simon Glass wrote:

Hi Heinrich,

On Sun, 6 Jun 2021 at 11:28, Heinrich Schuchardt  wrote:


On 6/6/21 6:44 PM, Simon Glass wrote:

Hi Heinrich,

On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:


Hi Heinrich,

On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt  wrote:


On 22.03.21 06:21, Simon Glass wrote:

At present if sandbox crashes it prints a message and tries to exit. But
with the recently introduced signal handler, it often seems to get stuck
in a loop until the stack overflows:

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation
...


Hello Simon,

do you have a reproducible example? I never have seen this.


https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433

You need to run that commit with pytest though...it does not happen
when run directly.

BTW this sems to expose some rather nasty bug in dlmalloc or how it is
used. I notice that as soon as the first test is run, the 'top' value
in dlmalloc is outside the range of the malloc pool, which seems
wrong. I wonder if there is something broken with how
dm_test_pre_run() and dm_test_post_run() work.



Corrupting gd could cause an endless recursive loop, as these lines
follow printing the observed string:

  printf("pc = 0x%lx, ", pc);
  printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);


Yes I suspect printf() is dead.



If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
recursion cannot occur anymore. If a segmentation violation occurs
inside the handler it will be delegated to the default handler.

Furthermore we could consider removing the signal handler at the start
of os_signal_action().


The issue is that if you get a segfault you really don't know if you
can continue and do anything else.

What is the goal with the signal handler? I don't think the user can
do anything about it.


Hello Simon,

the signal handler prints out the crash location and this makes
analyzing problems much easier. It proved valuable to me several times.


Well I think we are at a draw on that point, as the patch has caused
me pain many times!





I keep hitting this problem during development with sandbox, so I
think I need to apply this patch.

Does anything need to be updated in the tests?

Regards,
Simon



Did you try removing SA_NODEFER as proposed?


But what is the goal here...do you mean you want it to crash later? I
just want it to crash immediately.


If you have not messed up gd, U-Boot will print the program counter
address and the program will exit. You can try this with the exception
command.

If you have messed up gd, you will still exit but you will get a SIGSEGV
warning from the operating system.



What's actually wrong with putting this behaviour behind a flag? You
could always run with the flag enabled if needed. But I just don't
think it makes sense for the default behaviour to be to try to
continue operation.


I want to know the relocated PC counter when the sandbox crashes.
Why do you want to hide it by default?

Best regards

Heinrich


Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Simon Glass
Hi Heinrich,

On Sun, 6 Jun 2021 at 11:28, Heinrich Schuchardt  wrote:
>
> On 6/6/21 6:44 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:
> >>
> >> Hi Heinrich,
> >>
> >> On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt  
> >> wrote:
> >>>
> >>> On 22.03.21 06:21, Simon Glass wrote:
>  At present if sandbox crashes it prints a message and tries to exit. But
>  with the recently introduced signal handler, it often seems to get stuck
>  in a loop until the stack overflows:
> 
>  Segmentation violation
> 
>  Segmentation violation
> 
>  Segmentation violation
> 
>  Segmentation violation
> 
>  Segmentation violation
> 
>  Segmentation violation
> 
>  Segmentation violation
>  ...
> >>>
> >>> Hello Simon,
> >>>
> >>> do you have a reproducible example? I never have seen this.
> >>
> >> https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433
> >>
> >> You need to run that commit with pytest though...it does not happen
> >> when run directly.
> >>
> >> BTW this sems to expose some rather nasty bug in dlmalloc or how it is
> >> used. I notice that as soon as the first test is run, the 'top' value
> >> in dlmalloc is outside the range of the malloc pool, which seems
> >> wrong. I wonder if there is something broken with how
> >> dm_test_pre_run() and dm_test_post_run() work.
> >>
> >>>
> >>> Corrupting gd could cause an endless recursive loop, as these lines
> >>> follow printing the observed string:
> >>>
> >>>  printf("pc = 0x%lx, ", pc);
> >>>  printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);
> >>
> >> Yes I suspect printf() is dead.
> >>
> >>>
> >>> If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
> >>> recursion cannot occur anymore. If a segmentation violation occurs
> >>> inside the handler it will be delegated to the default handler.
> >>>
> >>> Furthermore we could consider removing the signal handler at the start
> >>> of os_signal_action().
> >>
> >> The issue is that if you get a segfault you really don't know if you
> >> can continue and do anything else.
> >>
> >> What is the goal with the signal handler? I don't think the user can
> >> do anything about it.
>
> Hello Simon,
>
> the signal handler prints out the crash location and this makes
> analyzing problems much easier. It proved valuable to me several times.

Well I think we are at a draw on that point, as the patch has caused
me pain many times!

>
> >
> > I keep hitting this problem during development with sandbox, so I
> > think I need to apply this patch.
> >
> > Does anything need to be updated in the tests?
> >
> > Regards,
> > Simon
> >
>
> Did you try removing SA_NODEFER as proposed?

But what is the goal here...do you mean you want it to crash later? I
just want it to crash immediately.

What's actually wrong with putting this behaviour behind a flag? You
could always run with the flag enabled if needed. But I just don't
think it makes sense for the default behaviour to be to try to
continue operation.

Regards,
Simon


Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Heinrich Schuchardt

On 6/6/21 6:44 PM, Simon Glass wrote:

Hi Heinrich,

On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:


Hi Heinrich,

On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt  wrote:


On 22.03.21 06:21, Simon Glass wrote:

At present if sandbox crashes it prints a message and tries to exit. But
with the recently introduced signal handler, it often seems to get stuck
in a loop until the stack overflows:

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation

Segmentation violation
...


Hello Simon,

do you have a reproducible example? I never have seen this.


https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433

You need to run that commit with pytest though...it does not happen
when run directly.

BTW this sems to expose some rather nasty bug in dlmalloc or how it is
used. I notice that as soon as the first test is run, the 'top' value
in dlmalloc is outside the range of the malloc pool, which seems
wrong. I wonder if there is something broken with how
dm_test_pre_run() and dm_test_post_run() work.



Corrupting gd could cause an endless recursive loop, as these lines
follow printing the observed string:

 printf("pc = 0x%lx, ", pc);
 printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);


Yes I suspect printf() is dead.



If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
recursion cannot occur anymore. If a segmentation violation occurs
inside the handler it will be delegated to the default handler.

Furthermore we could consider removing the signal handler at the start
of os_signal_action().


The issue is that if you get a segfault you really don't know if you
can continue and do anything else.

What is the goal with the signal handler? I don't think the user can
do anything about it.


Hello Simon,

the signal handler prints out the crash location and this makes
analyzing problems much easier. It proved valuable to me several times.



I keep hitting this problem during development with sandbox, so I
think I need to apply this patch.

Does anything need to be updated in the tests?

Regards,
Simon



Did you try removing SA_NODEFER as proposed?

Best regards

Heinrich


Re: [PATCH V5 1/2] mmc: add OpenPiton mmc support

2021-06-06 Thread Sean Anderson

On 6/6/21 12:57 PM, Tianrui Wei wrote:

Dear Sean,

Many thanks for taking the time and energy to review our patches ( again )
And sorry about not responding or not implementing them. I'll go through
them one by one and try to doing so in the future :P

On 6/7/21 12:34 AM, Sean Anderson wrote:

On 6/4/21 12:52 AM, Tianrui Wei wrote:

From: Tianrui Wei 
Date: Fri, 4 June 2021 12:45:29 +0800
Subject: [PATCH v5 1/2] mmc: add OpenPiton mmc support

This patch adds mmc support for OpenPiton board.

Changelog:

v5: move definitions around, changed some incompatible type information

Signed-off-by: Tianrui Wei 
Signed-off-by: Jonathan Balkind 
---

   drivers/mmc/Kconfig |   7 +
   drivers/mmc/Makefile|   1 +
   drivers/mmc/piton_mmc.c | 174 +
   3 files changed, 182 insertions(+)
   create mode 100644 drivers/mmc/piton_mmc.c

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 14d79139..1038800f 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -707,6 +707,13 @@ config MMC_SUNXI_HAS_MODE_SWITCH
   bool
   depends on MMC_SUNXI
   +config MMC_PITON
+bool "MMC support for openpiton SoC"
+  depends on DM_MMC && BLK
+help
+This driver enables SD card support in U-Boot port for


We already know it's for U-Boot ;)


:P




+OpenPiton SoC


Please add a bit more than this. In particular, it would be useful to
note that this is an "emulated" device where the host does all of the
work.


Will do :P




+
   config GENERIC_ATMEL_MCI
   bool "Atmel Multimedia Card Interface support"
   depends on DM_MMC && BLK && ARCH_AT91
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index 1c849cba..698dfe05 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_MMC_SDHCI_XENON)+= xenon_sdhci.o
   obj-$(CONFIG_MMC_SDHCI_ZYNQ)+= zynq_sdhci.o
 obj-$(CONFIG_MMC_SUNXI)+= sunxi_mmc.o
+obj-$(CONFIG_MMC_PITON) += piton_mmc.o
   obj-$(CONFIG_MMC_UNIPHIER)+= tmio-common.o uniphier-sd.o
   obj-$(CONFIG_RENESAS_SDHI)+= tmio-common.o renesas-sdhi.o
   obj-$(CONFIG_MMC_BCM2835)+= bcm2835_sdhost.o
diff --git a/drivers/mmc/piton_mmc.c b/drivers/mmc/piton_mmc.c
new file mode 100644
index ..2b2d5756
--- /dev/null
+++ b/drivers/mmc/piton_mmc.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2009 SAMSUNG Electronics
+ * Minkyu Kang 
+ * Jaehoon Chung 
+ * Portions Copyright 2011-2019 NVIDIA Corporation
+ * Portions Copyright 2021 Tianrui Wei
+ * This file is adapted from tegra_mmc.c
+ * Tianrui Wei 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct piton_mmc_plat {
+struct mmc_config cfg;
+struct mmc mmc;
+};
+
+struct piton_mmc_priv {
+u64 piton_mmc_base_addr; /* peripheral id */


Again, please use void __iomem *.


Sorry, forgot this change :(




+};
+
+/*
+ * see mmc_read_blocks to see how it is used.
+ * start block is hidden at cmd->arg


Hidden?


I wrote this comment a long time ago, I'll comment it out.




+ * also, initialize the block size at init
+ */
+static int piton_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
+  struct mmc_data *data)


Nit: alignment


I'm not sure how to align this? Should I align the struct in the first line
and the second line? It appears so in my email client :P


In your original email, this second line has 7 tabs, whereas the line
above it has 29 characters before the `(`. I'm not sure why it's aligned
in my reply. You can see the misaligned issue if you view your patch in
patchwork.






+{
+/* check first if this is a pure command */
+if (!data)
+return 0;


Variable declarations come before code.


Will do :P




+
+u64 byte_cnt, start_block, start_addr;


I think these should be size_t, since you use it for pointer
arithmetic.


Will do :P




+
+struct piton_mmc_priv *priv = dev_get_priv(dev);
+unsigned long *buff = (unsigned long *)data->dest;
+
+start_addr = priv->piton_mmc_base_addr + (start_block);
+byte_cnt = data->blocks * data->blocksize;


What should happen if (byte_cnt & 3)?


The 1th or 3rd byte will be read into buf,


Ah, disregard this; presumably blocksize will always be a (large enough)
power of two.






+start_block = cmd->cmdarg;
+
+/* if there is a read */
+if (data->flags & MMC_DATA_READ) {
+for (u64 i = 0; i < byte_cnt; i += 4) {


Declare i with the rest of the variables.


+*(buff) = readl((void *)(start_addr + i));


I think this could be rewritten as

size_t byte_cnt;
u32 *buff, *start_addr;

/* ... */

for (byte_cnt = /* ... */; byte_cnt; byte_cnt -= sizeof(u32));
 *buff++ = readl(start_addr++);

Which more closely follows the model of memcpy. Speaking of which, I
thought you were going to use it:


Thanks 

Re: [PATCH V5 2/2] riscv: board: Support OpenPiton SoC

2021-06-06 Thread Sean Anderson

On 6/4/21 12:54 AM, Tianrui Wei wrote:

From: Tianrui Wei 
Date: Fri, 4 June 2021 12:45:29 +0800
Subject: [PATCH v5 2/2] riscv: board: Support OpenPiton SoC


This patch does not apply cleanly. Your mail client may have wrapped
some of the longer lines.



This patch add board support for OpenPiton.


Please add a longer commit message. It is customary to describe the
major features of the SoC in the initial commit.



Changelog:

v5:
- major changes to device tree
- change defconfig to use OF_SEPARATE and other things
- change some documentation

Signed-off-by: Tianrui Wei 
Signed-off-by: Jonathan Balkind 
---

  arch/riscv/Kconfig  |   4 +
  arch/riscv/dts/Makefile |   1 +
  arch/riscv/dts/openpiton-riscv64.dts| 160 +
  board/openpiton/riscv/Kconfig   |  42 ++
  board/openpiton/riscv/MAINTAINERS   |   6 +
  board/openpiton/riscv/Makefile  |   5 +
  board/openpiton/riscv/openpiton-riscv.c |  41 ++
  configs/openpiton_riscv64_defconfig | 132 
  doc/board/index.rst |   1 +
  doc/board/openpiton/index.rst   |   9 +
  doc/board/openpiton/riscv64.rst | 885 
  include/configs/openpiton-riscv.h   |  58 ++
  12 files changed, 1344 insertions(+)
  create mode 100644 arch/riscv/dts/openpiton-riscv64.dts
  create mode 100644 board/openpiton/riscv/Kconfig
  create mode 100644 board/openpiton/riscv/MAINTAINERS
  create mode 100644 board/openpiton/riscv/Makefile
  create mode 100644 board/openpiton/riscv/openpiton-riscv.c
  create mode 100644 configs/openpiton_riscv64_defconfig
  create mode 100644 doc/board/openpiton/index.rst
  create mode 100644 doc/board/openpiton/riscv64.rst
  create mode 100644 include/configs/openpiton-riscv.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 30b05408..9e7deb34 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -23,6 +23,9 @@ config TARGET_SIFIVE_FU540
  config TARGET_SIPEED_MAIX
bool "Support Sipeed Maix Board"
  
+config TARGET_OPENPITON_RISCV

+  bool "Support riscv cores on openpiton SoC"


Is this the correct capitalization for OpenPiton?


+
  endchoice
  
  config SYS_ICACHE_OFF

@@ -55,6 +58,7 @@ config SPL_SYS_DCACHE_OFF
  source "board/AndesTech/ax25-ae350/Kconfig"
  source "board/emulation/qemu-riscv/Kconfig"
  source "board/microchip/mpfs_icicle/Kconfig"
+source "board/openpiton/riscv/Kconfig"
  source "board/sifive/fu540/Kconfig"
  source "board/sipeed/maix/Kconfig"
  
diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile

index 3a6f96c6..b511cd74 100644
--- a/arch/riscv/dts/Makefile
+++ b/arch/riscv/dts/Makefile
@@ -1,6 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0+
  
  dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb

+dtb-$(CONFIG_TARGET_OPENPITON_RISCV) += openpiton-riscv64.dtb
  dtb-$(CONFIG_TARGET_SIFIVE_FU540) += hifive-unleashed-a00.dtb
  dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
  
diff --git a/arch/riscv/dts/openpiton-riscv64.dts b/arch/riscv/dts/openpiton-riscv64.dts

new file mode 100644
index ..3abeaff8
--- /dev/null
+++ b/arch/riscv/dts/openpiton-riscv64.dts
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2021 Tianrui Wei  */
+
+/*
+ * This dts is for a dual core instance of OpenPiton+Ariane built
+ * to run on a Digilent Genesys 2 FPGA at 66.67MHz. These files
+ * are automatically generated by the OpenPiton build system and
+ * this configuration may not be what you need if your configuration
+ * is different from the below.
+ */
+
+/dts-v1/;
+
+/ {
+   #address-cells = <2>;
+   #size-cells = <2>;
+   u-boot,dm-spl;


Why is this here at the top-level?


+   compatible = "openpiton,riscv64";
+
+   chosen {
+  stdout-path = "uart0:115200";
+   };
+
+   aliases {
+   console = 
+   serial0 = 
+   };
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   timebase-frequency = <520835>;
+
+   CPU0: cpu@0 {
+   clocks = <>;
+   u-boot,dm-spl;
+   device_type = "cpu";
+   reg = <0>;
+   status = "okay";


This is unnecessary.


+   compatible = "openhwgroup,cva6", "riscv";
+   riscv,isa = "rv64imafdc";
+   mmu-type = "riscv,sv39";
+   tlb-split;
+   // HLIC - hart local interrupt controller
+   CPU0_intc: interrupt-controller {
+   #interrupt-cells = <1>;
+   interrupt-controller;
+   compatible = "riscv,cpu-intc";
+   };
+   };
+
+   CPU1: cpu@1 {
+   clocks = <>;
+   device_type = "cpu";
+   reg 

Re: [PATCH V5 1/2] mmc: add OpenPiton mmc support

2021-06-06 Thread Tianrui Wei
Dear Sean,

Many thanks for taking the time and energy to review our patches ( again )
And sorry about not responding or not implementing them. I'll go through
them one by one and try to doing so in the future :P

On 6/7/21 12:34 AM, Sean Anderson wrote:
> On 6/4/21 12:52 AM, Tianrui Wei wrote:
>> From: Tianrui Wei 
>> Date: Fri, 4 June 2021 12:45:29 +0800
>> Subject: [PATCH v5 1/2] mmc: add OpenPiton mmc support
>>
>> This patch adds mmc support for OpenPiton board.
>>
>> Changelog:
>>
>> v5: move definitions around, changed some incompatible type information
>>
>> Signed-off-by: Tianrui Wei 
>> Signed-off-by: Jonathan Balkind 
>> ---
>>
>>   drivers/mmc/Kconfig |   7 +
>>   drivers/mmc/Makefile    |   1 +
>>   drivers/mmc/piton_mmc.c | 174 +
>>   3 files changed, 182 insertions(+)
>>   create mode 100644 drivers/mmc/piton_mmc.c
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 14d79139..1038800f 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -707,6 +707,13 @@ config MMC_SUNXI_HAS_MODE_SWITCH
>>   bool
>>   depends on MMC_SUNXI
>>   +config MMC_PITON
>> +    bool "MMC support for openpiton SoC"
>> +  depends on DM_MMC && BLK
>> +    help
>> +    This driver enables SD card support in U-Boot port for
> 
> We already know it's for U-Boot ;)

:P

> 
>> +    OpenPiton SoC
> 
> Please add a bit more than this. In particular, it would be useful to
> note that this is an "emulated" device where the host does all of the
> work.

Will do :P

> 
>> +
>>   config GENERIC_ATMEL_MCI
>>   bool "Atmel Multimedia Card Interface support"
>>   depends on DM_MMC && BLK && ARCH_AT91
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 1c849cba..698dfe05 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -71,6 +71,7 @@ obj-$(CONFIG_MMC_SDHCI_XENON)    += xenon_sdhci.o
>>   obj-$(CONFIG_MMC_SDHCI_ZYNQ)    += zynq_sdhci.o
>>     obj-$(CONFIG_MMC_SUNXI)    += sunxi_mmc.o
>> +obj-$(CONFIG_MMC_PITON) += piton_mmc.o
>>   obj-$(CONFIG_MMC_UNIPHIER)    += tmio-common.o uniphier-sd.o
>>   obj-$(CONFIG_RENESAS_SDHI)    += tmio-common.o renesas-sdhi.o
>>   obj-$(CONFIG_MMC_BCM2835)    += bcm2835_sdhost.o
>> diff --git a/drivers/mmc/piton_mmc.c b/drivers/mmc/piton_mmc.c
>> new file mode 100644
>> index ..2b2d5756
>> --- /dev/null
>> +++ b/drivers/mmc/piton_mmc.c
>> @@ -0,0 +1,174 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2009 SAMSUNG Electronics
>> + * Minkyu Kang 
>> + * Jaehoon Chung 
>> + * Portions Copyright 2011-2019 NVIDIA Corporation
>> + * Portions Copyright 2021 Tianrui Wei
>> + * This file is adapted from tegra_mmc.c
>> + * Tianrui Wei 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +struct piton_mmc_plat {
>> +    struct mmc_config cfg;
>> +    struct mmc mmc;
>> +};
>> +
>> +struct piton_mmc_priv {
>> +    u64 piton_mmc_base_addr; /* peripheral id */
> 
> Again, please use void __iomem *.

Sorry, forgot this change :(

> 
>> +};
>> +
>> +/*
>> + * see mmc_read_blocks to see how it is used.
>> + * start block is hidden at cmd->arg
> 
> Hidden?

I wrote this comment a long time ago, I'll comment it out.

> 
>> + * also, initialize the block size at init
>> + */
>> +static int piton_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> +  struct mmc_data *data)
> 
> Nit: alignment

I'm not sure how to align this? Should I align the struct in the first line
and the second line? It appears so in my email client :P

> 
>> +{
>> +    /* check first if this is a pure command */
>> +    if (!data)
>> +    return 0;
> 
> Variable declarations come before code.

Will do :P

> 
>> +
>> +    u64 byte_cnt, start_block, start_addr;
> 
> I think these should be size_t, since you use it for pointer
> arithmetic.

Will do :P

> 
>> +
>> +    struct piton_mmc_priv *priv = dev_get_priv(dev);
>> +    unsigned long *buff = (unsigned long *)data->dest;
>> +
>> +    start_addr = priv->piton_mmc_base_addr + (start_block);
>> +    byte_cnt = data->blocks * data->blocksize;
> 
> What should happen if (byte_cnt & 3)?

The 1th or 3rd byte will be read into buf, 

> 
>> +    start_block = cmd->cmdarg;
>> +
>> +    /* if there is a read */
>> +    if (data->flags & MMC_DATA_READ) {
>> +    for (u64 i = 0; i < byte_cnt; i += 4) {
> 
> Declare i with the rest of the variables.
> 
>> +    *(buff) = readl((void *)(start_addr + i));
> 
> I think this could be rewritten as
> 
> size_t byte_cnt;
> u32 *buff, *start_addr;
> 
> /* ... */
> 
> for (byte_cnt = /* ... */; byte_cnt; byte_cnt -= sizeof(u32));
> *buff++ = readl(start_addr++);
> 
> Which more closely follows the model of memcpy. Speaking of which, I
> thought you were going to use it:


Re: BISECTED f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

2021-06-06 Thread Heinrich Schuchardt

On 6/6/21 6:21 PM, Heinrich Schuchardt wrote:

On 6/6/21 5:42 PM, Matwey V. Kornilov wrote:

вс, 6 июн. 2021 г. в 18:20, Heinrich Schuchardt :


On 6/6/21 4:37 PM, Matwey V. Kornilov wrote:

Hi,

I've found that

f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having
/EFI/boot")

breaks running EFI application from USB device on BeagleBone Black
(am335x) device.

With this patch I see the following:

Booting /efi\boot\bootarm.efi
Welcome to GRUB!

data abort
pc : [<9ce0b6d0>]  lr : [<9ffab7c7>]
reloc pc : [<7d69d6d0>]    lr : [<8083d7c7>]
sp : 9df44e28  ip : 9ffdfe90 fp : 0003
r10: 9ffe3300  r9 :  r8 : 9df6fe88
r7 :   r6 : 9ce5da08 r5 : 9ce571f8  r4 : 9ce2c040
r3 :   r2 : 0001 r1 : 9ce56598  r0 : 
Flags: NzCv  IRQs off  FIQs on  Mode SVC_32
Code: e350 0a15 e59c eb00f96e (e5d03000)

  > UEFI image [0x9ce46000:0x9cf28fff] '/efi\boot\bootarm.efi'
  > Resetting CPU ...

Hello Matwey,

thank you for reporting the issue.

$ echo 'Code: e350 0a15 e59c eb00f96e (e5d03000)' |
CROSS_COMPILE=arm-linux-gnueabihf- ARCH=arm scripts/decodecode

Code: e350 0a15 e59c eb00f96e (e5d03000)
All code

 0:   e350    cmp r0, #0
 4:   0a15    beq 0x60
 8:   e59c    ldr r0, [r0, #12]
 c:   eb00f96e    bl  0x3e5cc
    10:*  e5d03000    ldrb    r3, [r0]    <-- trapping
instruction

Code starting with the faulting instruction
===
 0:   e5d03000    ldrb    r3, [r0]

Looking at the disassembly above we see that reading memory location
NULL fails.

We need to find out where the exception occurs. The code position is
neither in bootarm.efi nor in U-Boot (9ce0b6d0 is lower than the load
position of bootarm.efi, so it is below the relocated U-Boot code).

Please, add the following line at the start of grub.cfg to get more
output from GRUB:

 debug=all


This doesn't provide any additional output from GRUB :(



When building U-Boot, please, add

 #define DEBUG 1

in lib/efi_loader/efi_disk.c and lib/efi_loader_file.c a line before
#include .



This doesn't provide much output as well:

Scanning disk m...@4806.blk...
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
** Unrecognized filesystem type **
Scanning disk m...@481d8000.blk...
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
Found 3 disks


This implies that GRUB is crashing before even accessing the file system
(including grub.cfg).

On an OrangePi PC I deleted /boot.scr and moved grubarm.efi to
/EFI/boot/bootarm.efi. It boots without problem.

What version of GRUB are you using?
How were you booting before updating U-Boot?
What version of U-Boot are you using where the error occurs?
Why do you have grub in /EFI/boot/bootarm.efi and not in a distro
specific path, e.g. /EFI/debian/grubarm.efi? /EFI/boot is typically only
used by installers.

If the boot manager is started by distroboot it may not have an
appropriate device path. It tries to load the file given by environment
variable $fdtfile from the boot device.

 From the U-Boot console could you, please, try:

1)
load usb 0:1 $kernel_addr_r EFI/boot/bootarm.efi
bootefi bootmgr


2)
load usb 0:1 $kernel_addr_r EFI/boot/bootarm.efi
load usb 0:2 $fdt_addr_r dtb
bootefi bootmgr $fdt_addr_r

where you need to replace dtb by the correct device tree file and adjust
the partition numbers.

Best regards

Heinrich


To catch the earlier EFI API calls you can add

#define DEBUG 1

to lib/efi_loader/efi_boottime.c

Best regards

Heinrich




Re: [PATCH] sandbox: Support signal handling only when requested

2021-06-06 Thread Simon Glass
Hi Heinrich,

On Mon, 22 Mar 2021 at 18:56, Simon Glass  wrote:
>
> Hi Heinrich,
>
> On Mon, 22 Mar 2021 at 23:02, Heinrich Schuchardt  wrote:
> >
> > On 22.03.21 06:21, Simon Glass wrote:
> > > At present if sandbox crashes it prints a message and tries to exit. But
> > > with the recently introduced signal handler, it often seems to get stuck
> > > in a loop until the stack overflows:
> > >
> > > Segmentation violation
> > >
> > > Segmentation violation
> > >
> > > Segmentation violation
> > >
> > > Segmentation violation
> > >
> > > Segmentation violation
> > >
> > > Segmentation violation
> > >
> > > Segmentation violation
> > > ...
> >
> > Hello Simon,
> >
> > do you have a reproducible example? I never have seen this.
>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/242433
>
> You need to run that commit with pytest though...it does not happen
> when run directly.
>
> BTW this sems to expose some rather nasty bug in dlmalloc or how it is
> used. I notice that as soon as the first test is run, the 'top' value
> in dlmalloc is outside the range of the malloc pool, which seems
> wrong. I wonder if there is something broken with how
> dm_test_pre_run() and dm_test_post_run() work.
>
> >
> > Corrupting gd could cause an endless recursive loop, as these lines
> > follow printing the observed string:
> >
> > printf("pc = 0x%lx, ", pc);
> > printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);
>
> Yes I suspect printf() is dead.
>
> >
> > If we remove SA_NODEFER from the signal mask in arch/sandbox/cpu/os.c,
> > recursion cannot occur anymore. If a segmentation violation occurs
> > inside the handler it will be delegated to the default handler.
> >
> > Furthermore we could consider removing the signal handler at the start
> > of os_signal_action().
>
> The issue is that if you get a segfault you really don't know if you
> can continue and do anything else.
>
> What is the goal with the signal handler? I don't think the user can
> do anything about it.

I keep hitting this problem during development with sandbox, so I
think I need to apply this patch.

Does anything need to be updated in the tests?

Regards,
Simon


Please pull u-boot-dm

2021-06-06 Thread Simon Glass
Hi Tom,

https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/7711


The following changes since commit c003d2cd6b415319c3e40db42b48c2815acda3d3:

  Merge https://source.denx.de/u-boot/custodians/u-boot-marvell (2021-06-04
09:34:21 -0400)

are available in the Git repository at:

  git://git.denx.de/u-boot-dm.git tags/dm-pull-6jun21

for you to fetch changes up to 269fa8468df40952d538ac945db3d98a8d44f79e:

  test: add dm_test_read_resource (2021-06-05 07:35:47 -0600)


Minor fixes for sandbox and handling of dm-ranges


Alper Nebi Yasak (1):
  pwm: cros_ec: Rename "priv_auto_alloc_size" to "priv_auto"

Bin Meng (2):
  of: addr: Translate 'dma-ranges' for parent nodes missing 'dma-ranges'
  of: addr: Remove call to dev_count_cells() in of_get_address()

Heinrich Schuchardt (1):
  sandbox: correct determination of the text base

Patrick Delaunay (2):
  net: luton: remove address translation after ofnode_read_resource
  test: add dm_test_read_resource

 arch/sandbox/cpu/start.c|  5 -
 drivers/core/of_addr.c  | 12 +---
 drivers/net/mscc_eswitch/luton_switch.c |  5 +
 drivers/pwm/cros_ec_pwm.c   |  2 +-
 test/dm/test-fdt.c  | 33
+
 5 files changed, 44 insertions(+), 13 deletions(-)

Regards,
Simon


Re: [PATCH V5 1/2] mmc: add OpenPiton mmc support

2021-06-06 Thread Sean Anderson

On 6/4/21 12:52 AM, Tianrui Wei wrote:

From: Tianrui Wei 
Date: Fri, 4 June 2021 12:45:29 +0800
Subject: [PATCH v5 1/2] mmc: add OpenPiton mmc support

This patch adds mmc support for OpenPiton board.

Changelog:

v5: move definitions around, changed some incompatible type information

Signed-off-by: Tianrui Wei 
Signed-off-by: Jonathan Balkind 
---

  drivers/mmc/Kconfig |   7 +
  drivers/mmc/Makefile|   1 +
  drivers/mmc/piton_mmc.c | 174 +
  3 files changed, 182 insertions(+)
  create mode 100644 drivers/mmc/piton_mmc.c

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 14d79139..1038800f 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -707,6 +707,13 @@ config MMC_SUNXI_HAS_MODE_SWITCH
bool
depends on MMC_SUNXI
  
+config MMC_PITON

+   bool "MMC support for openpiton SoC"
+  depends on DM_MMC && BLK
+   help
+This driver enables SD card support in U-Boot port for


We already know it's for U-Boot ;)


+OpenPiton SoC


Please add a bit more than this. In particular, it would be useful to
note that this is an "emulated" device where the host does all of the
work.


+
  config GENERIC_ATMEL_MCI
bool "Atmel Multimedia Card Interface support"
depends on DM_MMC && BLK && ARCH_AT91
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index 1c849cba..698dfe05 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_MMC_SDHCI_XENON) += xenon_sdhci.o
  obj-$(CONFIG_MMC_SDHCI_ZYNQ)  += zynq_sdhci.o
  
  obj-$(CONFIG_MMC_SUNXI)			+= sunxi_mmc.o

+obj-$(CONFIG_MMC_PITON)+= piton_mmc.o
  obj-$(CONFIG_MMC_UNIPHIER)+= tmio-common.o uniphier-sd.o
  obj-$(CONFIG_RENESAS_SDHI)+= tmio-common.o renesas-sdhi.o
  obj-$(CONFIG_MMC_BCM2835) += bcm2835_sdhost.o
diff --git a/drivers/mmc/piton_mmc.c b/drivers/mmc/piton_mmc.c
new file mode 100644
index ..2b2d5756
--- /dev/null
+++ b/drivers/mmc/piton_mmc.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2009 SAMSUNG Electronics
+ * Minkyu Kang 
+ * Jaehoon Chung 
+ * Portions Copyright 2011-2019 NVIDIA Corporation
+ * Portions Copyright 2021 Tianrui Wei
+ * This file is adapted from tegra_mmc.c
+ * Tianrui Wei 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct piton_mmc_plat {
+   struct mmc_config cfg;
+   struct mmc mmc;
+};
+
+struct piton_mmc_priv {
+   u64 piton_mmc_base_addr; /* peripheral id */


Again, please use void __iomem *.


+};
+
+/*
+ * see mmc_read_blocks to see how it is used.
+ * start block is hidden at cmd->arg


Hidden?


+ * also, initialize the block size at init
+ */
+static int piton_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
+ struct mmc_data *data)


Nit: alignment


+{
+   /* check first if this is a pure command */
+   if (!data)
+   return 0;


Variable declarations come before code.


+
+   u64 byte_cnt, start_block, start_addr;


I think these should be size_t, since you use it for pointer
arithmetic.


+
+   struct piton_mmc_priv *priv = dev_get_priv(dev);
+   unsigned long *buff = (unsigned long *)data->dest;
+
+   start_addr = priv->piton_mmc_base_addr + (start_block);
+   byte_cnt = data->blocks * data->blocksize;


What should happen if (byte_cnt & 3)?


+   start_block = cmd->cmdarg;
+
+   /* if there is a read */
+   if (data->flags & MMC_DATA_READ) {
+   for (u64 i = 0; i < byte_cnt; i += 4) {


Declare i with the rest of the variables.


+   *(buff) = readl((void *)(start_addr + i));


I think this could be rewritten as

size_t byte_cnt;
u32 *buff, *start_addr;

/* ... */

for (byte_cnt = /* ... */; byte_cnt; byte_cnt -= sizeof(u32));
*buff++ = readl(start_addr++);

Which more closely follows the model of memcpy. Speaking of which, I
thought you were going to use it:


Scratch the previous email, I stand corrected. You're right about
memcpy could work, thanks for the great suggestion :P 


What happened to that?


+   buff++;
+   }
+   } else {
+   /* else there is a write
+* we don't handle write, so error right away
+*/
+   return -ENODEV;


Please use -ENOSYS. See [1] for details.

In general, several of my comments on your previous patch are still
unaddressed. I would like if you could either respond to them with your
reasoning for not addressing them, or implement them.

--Sean


[1] 
https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#error-codes


+   }
+
+   return 0;
+}
+
+static int piton_mmc_ofdata_to_platdata(struct udevice *dev)
+{
+   struct piton_mmc_priv *priv = 

Re: BISECTED f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

2021-06-06 Thread Heinrich Schuchardt

On 6/6/21 5:42 PM, Matwey V. Kornilov wrote:

вс, 6 июн. 2021 г. в 18:20, Heinrich Schuchardt :


On 6/6/21 4:37 PM, Matwey V. Kornilov wrote:

Hi,

I've found that

f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

breaks running EFI application from USB device on BeagleBone Black
(am335x) device.

With this patch I see the following:

Booting /efi\boot\bootarm.efi
Welcome to GRUB!

data abort
pc : [<9ce0b6d0>]  lr : [<9ffab7c7>]
reloc pc : [<7d69d6d0>]lr : [<8083d7c7>]
sp : 9df44e28  ip : 9ffdfe90 fp : 0003
r10: 9ffe3300  r9 :  r8 : 9df6fe88
r7 :   r6 : 9ce5da08 r5 : 9ce571f8  r4 : 9ce2c040
r3 :   r2 : 0001 r1 : 9ce56598  r0 : 
Flags: NzCv  IRQs off  FIQs on  Mode SVC_32
Code: e350 0a15 e59c eb00f96e (e5d03000)

  > UEFI image [0x9ce46000:0x9cf28fff] '/efi\boot\bootarm.efi'
  > Resetting CPU ...

Hello Matwey,

thank you for reporting the issue.

$ echo 'Code: e350 0a15 e59c eb00f96e (e5d03000)' |
CROSS_COMPILE=arm-linux-gnueabihf- ARCH=arm scripts/decodecode

Code: e350 0a15 e59c eb00f96e (e5d03000)
All code

 0:   e350cmp r0, #0
 4:   0a15beq 0x60
 8:   e59cldr r0, [r0, #12]
 c:   eb00f96ebl  0x3e5cc
10:*  e5d03000ldrbr3, [r0]<-- trapping
instruction

Code starting with the faulting instruction
===
 0:   e5d03000ldrbr3, [r0]

Looking at the disassembly above we see that reading memory location
NULL fails.

We need to find out where the exception occurs. The code position is
neither in bootarm.efi nor in U-Boot (9ce0b6d0 is lower than the load
position of bootarm.efi, so it is below the relocated U-Boot code).

Please, add the following line at the start of grub.cfg to get more
output from GRUB:

 debug=all


This doesn't provide any additional output from GRUB :(



When building U-Boot, please, add

 #define DEBUG 1

in lib/efi_loader/efi_disk.c and lib/efi_loader_file.c a line before
#include .



This doesn't provide much output as well:

Scanning disk m...@4806.blk...
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
** Unrecognized filesystem type **
Scanning disk m...@481d8000.blk...
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
Found 3 disks


This implies that GRUB is crashing before even accessing the file system
(including grub.cfg).

On an OrangePi PC I deleted /boot.scr and moved grubarm.efi to
/EFI/boot/bootarm.efi. It boots without problem.

What version of GRUB are you using?
How were you booting before updating U-Boot?
What version of U-Boot are you using where the error occurs?
Why do you have grub in /EFI/boot/bootarm.efi and not in a distro
specific path, e.g. /EFI/debian/grubarm.efi? /EFI/boot is typically only
used by installers.

If the boot manager is started by distroboot it may not have an
appropriate device path. It tries to load the file given by environment
variable $fdtfile from the boot device.

From the U-Boot console could you, please, try:

1)
load usb 0:1 $kernel_addr_r EFI/boot/bootarm.efi
bootefi bootmgr


2)
load usb 0:1 $kernel_addr_r EFI/boot/bootarm.efi
load usb 0:2 $fdt_addr_r dtb
bootefi bootmgr $fdt_addr_r

where you need to replace dtb by the correct device tree file and adjust
the partition numbers.

Best regards

Heinrich


Re: BISECTED f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

2021-06-06 Thread Matwey V. Kornilov
вс, 6 июн. 2021 г. в 18:20, Heinrich Schuchardt :
>
> On 6/6/21 4:37 PM, Matwey V. Kornilov wrote:
> > Hi,
> >
> > I've found that
> >
> > f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having 
> > /EFI/boot")
> >
> > breaks running EFI application from USB device on BeagleBone Black
> > (am335x) device.
> >
> > With this patch I see the following:
> >
> > Booting /efi\boot\bootarm.efi
> > Welcome to GRUB!
> >
> > data abort
> > pc : [<9ce0b6d0>]  lr : [<9ffab7c7>]
> > reloc pc : [<7d69d6d0>]lr : [<8083d7c7>]
> > sp : 9df44e28  ip : 9ffdfe90 fp : 0003
> > r10: 9ffe3300  r9 :  r8 : 9df6fe88
> > r7 :   r6 : 9ce5da08 r5 : 9ce571f8  r4 : 9ce2c040
> > r3 :   r2 : 0001 r1 : 9ce56598  r0 : 
> > Flags: NzCv  IRQs off  FIQs on  Mode SVC_32
> > Code: e350 0a15 e59c eb00f96e (e5d03000)
>  > UEFI image [0x9ce46000:0x9cf28fff] '/efi\boot\bootarm.efi'
>  > Resetting CPU ...
>
> Hello Matwey,
>
> thank you for reporting the issue.
>
> $ echo 'Code: e350 0a15 e59c eb00f96e (e5d03000)' |
> CROSS_COMPILE=arm-linux-gnueabihf- ARCH=arm scripts/decodecode
>
> Code: e350 0a15 e59c eb00f96e (e5d03000)
> All code
> 
> 0:   e350cmp r0, #0
> 4:   0a15beq 0x60
> 8:   e59cldr r0, [r0, #12]
> c:   eb00f96ebl  0x3e5cc
>10:*  e5d03000ldrbr3, [r0]<-- trapping
> instruction
>
> Code starting with the faulting instruction
> ===
> 0:   e5d03000ldrbr3, [r0]
>
> Looking at the disassembly above we see that reading memory location
> NULL fails.
>
> We need to find out where the exception occurs. The code position is
> neither in bootarm.efi nor in U-Boot (9ce0b6d0 is lower than the load
> position of bootarm.efi, so it is below the relocated U-Boot code).
>
> Please, add the following line at the start of grub.cfg to get more
> output from GRUB:
>
> debug=all

This doesn't provide any additional output from GRUB :(

>
> When building U-Boot, please, add
>
> #define DEBUG 1
>
> in lib/efi_loader/efi_disk.c and lib/efi_loader_file.c a line before
> #include .


This doesn't provide much output as well:

Scanning disk m...@4806.blk...
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
** Unrecognized filesystem type **
Scanning disk m...@481d8000.blk...
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: Call: efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
EFI: 0 returned by efi_install_multiple_protocol_interfaces( ,
_guid_device_path, diskobj->dp, _block_io_guid, >ops,
NULL)
Found 3 disks


> Best regards
>
> Heinrich
>
> >
> > while without the patch, GRUB works as usual.
> > Could you please help me to figure out what is going wrong here?
> >



-- 
With best regards,
Matwey V. Kornilov


Re: BISECTED f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

2021-06-06 Thread Heinrich Schuchardt

On 6/6/21 4:37 PM, Matwey V. Kornilov wrote:

Hi,

I've found that

f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

breaks running EFI application from USB device on BeagleBone Black
(am335x) device.

With this patch I see the following:

Booting /efi\boot\bootarm.efi
Welcome to GRUB!

data abort
pc : [<9ce0b6d0>]  lr : [<9ffab7c7>]
reloc pc : [<7d69d6d0>]lr : [<8083d7c7>]
sp : 9df44e28  ip : 9ffdfe90 fp : 0003
r10: 9ffe3300  r9 :  r8 : 9df6fe88
r7 :   r6 : 9ce5da08 r5 : 9ce571f8  r4 : 9ce2c040
r3 :   r2 : 0001 r1 : 9ce56598  r0 : 
Flags: NzCv  IRQs off  FIQs on  Mode SVC_32
Code: e350 0a15 e59c eb00f96e (e5d03000)

> UEFI image [0x9ce46000:0x9cf28fff] '/efi\boot\bootarm.efi'
> Resetting CPU ...

Hello Matwey,

thank you for reporting the issue.

$ echo 'Code: e350 0a15 e59c eb00f96e (e5d03000)' |
CROSS_COMPILE=arm-linux-gnueabihf- ARCH=arm scripts/decodecode

Code: e350 0a15 e59c eb00f96e (e5d03000)
All code

   0:   e350cmp r0, #0
   4:   0a15beq 0x60
   8:   e59cldr r0, [r0, #12]
   c:   eb00f96ebl  0x3e5cc
  10:*  e5d03000ldrbr3, [r0]<-- trapping
instruction

Code starting with the faulting instruction
===
   0:   e5d03000ldrbr3, [r0]

Looking at the disassembly above we see that reading memory location
NULL fails.

We need to find out where the exception occurs. The code position is
neither in bootarm.efi nor in U-Boot (9ce0b6d0 is lower than the load
position of bootarm.efi, so it is below the relocated U-Boot code).

Please, add the following line at the start of grub.cfg to get more
output from GRUB:

debug=all

When building U-Boot, please, add

#define DEBUG 1

in lib/efi_loader/efi_disk.c and lib/efi_loader_file.c a line before
#include .

Best regards

Heinrich



while without the patch, GRUB works as usual.
Could you please help me to figure out what is going wrong here?



BISECTED f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

2021-06-06 Thread Matwey V. Kornilov
Hi,

I've found that

f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")

breaks running EFI application from USB device on BeagleBone Black
(am335x) device.

With this patch I see the following:

Booting /efi\boot\bootarm.efi
Welcome to GRUB!

data abort
pc : [<9ce0b6d0>]  lr : [<9ffab7c7>]
reloc pc : [<7d69d6d0>]lr : [<8083d7c7>]
sp : 9df44e28  ip : 9ffdfe90 fp : 0003
r10: 9ffe3300  r9 :  r8 : 9df6fe88
r7 :   r6 : 9ce5da08 r5 : 9ce571f8  r4 : 9ce2c040
r3 :   r2 : 0001 r1 : 9ce56598  r0 : 
Flags: NzCv  IRQs off  FIQs on  Mode SVC_32
Code: e350 0a15 e59c eb00f96e (e5d03000)
UEFI image [0x9ce46000:0x9cf28fff] '/efi\boot\bootarm.efi'
Resetting CPU ...

while without the patch, GRUB works as usual.
Could you please help me to figure out what is going wrong here?

-- 
With best regards,
Matwey V. Kornilov