[PATCH 3/5] arm: dts: rockchip: add Radxa ROCK 4C+

2023-04-25 Thread FUKAUMI Naoki
Linux commit 246450344dad arm64: dts: rockchip: rk3399: Radxa ROCK 4C+

Add support for Radxa ROCK 4C+ SBC.

Key differences of 4C+ compared to previous ROCK Pi 4.
- Rockchip RK3399-T SoC
- DP from 4C replaced with micro HDMI 2K@60fps
- 4-lane MIPI DSI with 1920*1080
- RK817 Audio codec

Also, an official naming convention from Radxa mention to remove
Pi from board name, so this 4C+ is named as Radxa ROCK 4C+ not
Radxa ROCK Pi 4C+.

Signed-off-by: Stephen Chen 
Signed-off-by: Manoj Sai 
Signed-off-by: Jagan Teki 
Signed-off-by: FUKAUMI Naoki 
---
 arch/arm/dts/Makefile|   1 +
 arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi |   5 +
 arch/arm/dts/rk3399-rock-4c-plus.dts | 709 +++
 arch/arm/dts/rk3399-t-opp.dtsi   | 114 +++
 4 files changed, 829 insertions(+)
 create mode 100644 arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-rock-4c-plus.dts
 create mode 100644 arch/arm/dts/rk3399-t-opp.dtsi

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 1d674b90af..2e0dbf8720 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -157,6 +157,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3399) += \
rk3399-puma-haikou.dtb \
rk3399-roc-pc.dtb \
rk3399-roc-pc-mezzanine.dtb \
+   rk3399-rock-4c-plus.dtb \
rk3399-rock-pi-4a.dtb \
rk3399-rock-pi-4c.dtb \
rk3399-rock960.dtb \
diff --git a/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi 
b/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
new file mode 100644
index 00..5c1c451b8f
--- /dev/null
+++ b/arch/arm/dts/rk3399-rock-4c-plus-u-boot.dtsi
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2023 Radxa Limited
+ */
+#include "rk3399-rock-pi-4-u-boot.dtsi"
diff --git a/arch/arm/dts/rk3399-rock-4c-plus.dts 
b/arch/arm/dts/rk3399-rock-4c-plus.dts
new file mode 100644
index 00..028eb508ae
--- /dev/null
+++ b/arch/arm/dts/rk3399-rock-4c-plus.dts
@@ -0,0 +1,709 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Fuzhou Rockchip Electronics Co., Ltd
+ * Copyright (c) 2019 Radxa Limited
+ * Copyright (c) 2022 Amarula Solutions(India)
+ */
+
+/dts-v1/;
+#include 
+#include "rk3399.dtsi"
+#include "rk3399-t-opp.dtsi"
+
+/ {
+   model = "Radxa ROCK 4C+";
+   compatible = "radxa,rock-4c-plus", "rockchip,rk3399";
+
+   aliases {
+   mmc0 = 
+   mmc1 = 
+   };
+
+   chosen {
+   stdout-path = "serial2:150n8";
+   };
+
+   clkin_gmac: external-gmac-clock {
+   compatible = "fixed-clock";
+   clock-frequency = <12500>;
+   clock-output-names = "clkin_gmac";
+   #clock-cells = <0>;
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_led1 _led2>;
+
+   /* USER_LED1 */
+   led-0 {
+   function = LED_FUNCTION_POWER;
+   color = ;
+   gpios = < RK_PD4 GPIO_ACTIVE_LOW>;
+   linux,default-trigger = "default-on";
+   };
+
+   /* USER_LED2 */
+   led-1 {
+   function = LED_FUNCTION_STATUS;
+   color = ;
+   gpios = < RK_PD5 GPIO_ACTIVE_HIGH>;
+   linux,default-trigger = "heartbeat";
+   };
+   };
+
+   sdio_pwrseq: sdio-pwrseq {
+   compatible = "mmc-pwrseq-simple";
+   clocks = < 1>;
+   clock-names = "ext_clock";
+   pinctrl-names = "default";
+   pinctrl-0 = <_enable_h>;
+   reset-gpios = < RK_PB2 GPIO_ACTIVE_LOW>;
+   };
+
+   vcc_3v3: vcc-3v3-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "vcc_3v3";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   vin-supply = <_sys>;
+   };
+
+   vcc3v3_phy1: vcc3v3-phy1-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "vcc3v3_phy1";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   vin-supply = <_3v3>;
+   };
+
+   vcc5v0_host1: vcc5v0-host-regulator {
+   compatible = "regulator-fixed";
+   enable-active-high;
+   gpio = < RK_PD6 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_host_en>;
+   regulator-name = "vcc5v0_host1";
+   regulator-always-on;
+   regulator-boot-on;
+   vin-supply = <_host0_s0>;
+   

[PATCH 1/5] arm: dts: rockchip: rock-pi-4: sync with Linux 6.3

2023-04-25 Thread FUKAUMI Naoki
sync dts{,i} files for Radxa ROCK Pi 4 series with Linux 6.3.

because rk3399-rock-pi-4a.dts is enough for ROCK Pi 4A/B/A+/B+ and ROCK
4SE, delete dts{,i} for ROCK Pi 4B.

Signed-off-by: FUKAUMI Naoki 
---
 arch/arm/dts/Makefile |   1 -
 arch/arm/dts/rk3399-rock-pi-4.dtsi| 229 --
 ...oot.dtsi => rk3399-rock-pi-4a-u-boot.dtsi} |   0
 arch/arm/dts/rk3399-rock-pi-4b.dts|  46 
 arch/arm/dts/rk3399-rock-pi-4c.dts|  20 +-
 5 files changed, 174 insertions(+), 122 deletions(-)
 rename arch/arm/dts/{rk3399-rock-pi-4b-u-boot.dtsi => 
rk3399-rock-pi-4a-u-boot.dtsi} (100%)
 delete mode 100644 arch/arm/dts/rk3399-rock-pi-4b.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 3385948d22..1d674b90af 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -158,7 +158,6 @@ dtb-$(CONFIG_ROCKCHIP_RK3399) += \
rk3399-roc-pc.dtb \
rk3399-roc-pc-mezzanine.dtb \
rk3399-rock-pi-4a.dtb \
-   rk3399-rock-pi-4b.dtb \
rk3399-rock-pi-4c.dtb \
rk3399-rock960.dtb \
rk3399-rockpro64.dtb \
diff --git a/arch/arm/dts/rk3399-rock-pi-4.dtsi 
b/arch/arm/dts/rk3399-rock-pi-4.dtsi
index b2ea92..907071d4fe 100644
--- a/arch/arm/dts/rk3399-rock-pi-4.dtsi
+++ b/arch/arm/dts/rk3399-rock-pi-4.dtsi
@@ -6,14 +6,15 @@
 
 /dts-v1/;
 #include 
+#include 
 #include 
 #include "rk3399.dtsi"
 #include "rk3399-opp.dtsi"
 
 / {
aliases {
-   mmc0 = 
-   mmc1 = 
+   mmc0 = 
+   mmc1 = 
};
 
chosen {
@@ -27,6 +28,20 @@
#clock-cells = <0>;
};
 
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_led2>;
+
+   /* USER_LED2 */
+   led-0 {
+   function = LED_FUNCTION_STATUS;
+   color = ;
+   gpios = < RK_PD5 GPIO_ACTIVE_HIGH>;
+   linux,default-trigger = "heartbeat";
+   };
+   };
+
sdio_pwrseq: sdio-pwrseq {
compatible = "mmc-pwrseq-simple";
clocks = < 1>;
@@ -36,32 +51,56 @@
reset-gpios = < RK_PB2 GPIO_ACTIVE_LOW>;
};
 
-   vcc12v_dcin: dc-12v {
+   sound: sound {
+   compatible = "audio-graph-card";
+   label = "Analog";
+   dais = <_p0>;
+   };
+
+   sound-dit {
+   compatible = "audio-graph-card";
+   label = "SPDIF";
+   dais = <_p0>;
+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;
+
+   port {
+   dit_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
+
+   vbus_typec: vbus-typec-regulator {
compatible = "regulator-fixed";
-   regulator-name = "vcc12v_dcin";
+   enable-active-high;
+   gpio = < RK_PA3 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_typec_en>;
+   regulator-name = "vbus_typec";
regulator-always-on;
-   regulator-boot-on;
-   regulator-min-microvolt = <1200>;
-   regulator-max-microvolt = <1200>;
+   vin-supply = <_sys>;
};
 
-   vcc5v0_sys: vcc-sys {
+   vcc12v_dcin: dc-12v {
compatible = "regulator-fixed";
-   regulator-name = "vcc5v0_sys";
+   regulator-name = "vcc12v_dcin";
regulator-always-on;
regulator-boot-on;
-   regulator-min-microvolt = <500>;
-   regulator-max-microvolt = <500>;
-   vin-supply = <_dcin>;
+   regulator-min-microvolt = <1200>;
+   regulator-max-microvolt = <1200>;
};
 
-   vcc_0v9: vcc-0v9 {
+   vcc3v3_lan: vcc3v3-lan-regulator {
compatible = "regulator-fixed";
-   regulator-name = "vcc_0v9";
+   regulator-name = "vcc3v3_lan";
regulator-always-on;
regulator-boot-on;
-   regulator-min-microvolt = <90>;
-   regulator-max-microvolt = <90>;
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
vin-supply = <_sys>;
};
 
@@ -98,35 +137,35 @@
vin-supply = <_sys>;
};
 
-   vcc5v0_typec: vcc5v0-typec-regulator {
+   vcc5v0_sys: vcc-sys {
compatible = "regulator-fixed";
-   enable-active-high;
-   gpio = < RK_PA3 GPIO_ACTIVE_HIGH>;
-   pinctrl-names = "default";
-   pinctrl-0 = <_typec_en>;
-   

[PATCH 4/5] configs: rockchip: add Radxa ROCK 4C+

2023-04-25 Thread FUKAUMI Naoki
add defconfig for Radxa ROCK 4C+.

Signed-off-by: FUKAUMI Naoki 
---
 configs/rock-4c-plus-rk3399_defconfig | 97 +++
 1 file changed, 97 insertions(+)
 create mode 100644 configs/rock-4c-plus-rk3399_defconfig

diff --git a/configs/rock-4c-plus-rk3399_defconfig 
b/configs/rock-4c-plus-rk3399_defconfig
new file mode 100644
index 00..97693166d9
--- /dev/null
+++ b/configs/rock-4c-plus-rk3399_defconfig
@@ -0,0 +1,97 @@
+CONFIG_ARM=y
+CONFIG_SKIP_LOWLEVEL_INIT=y
+CONFIG_COUNTER_FREQUENCY=2400
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_TEXT_BASE=0x0020
+CONFIG_NR_DRAM_BANKS=1
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
+CONFIG_ENV_OFFSET=0x3F8000
+CONFIG_DEFAULT_DEVICE_TREE="rk3399-rock-4c-plus"
+CONFIG_DM_RESET=y
+CONFIG_ROCKCHIP_RK3399=y
+CONFIG_TARGET_EVB_RK3399=y
+CONFIG_SPL_STACK=0x40
+CONFIG_DEBUG_UART_BASE=0xFF1A
+CONFIG_DEBUG_UART_CLOCK=2400
+CONFIG_SYS_LOAD_ADDR=0x800800
+CONFIG_DEBUG_UART=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-4c-plus.dtb"
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_MISC_INIT_R=y
+CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_PAD_TO=0x7f8000
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x40
+CONFIG_SPL_BSS_MAX_SIZE=0x2000
+# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
+CONFIG_TPL=y
+CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_NVEDIT_EFI=y
+CONFIG_CMD_DFU=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_ROCKUSB=y
+CONFIG_CMD_USB_MASS_STORAGE=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_EFIDEBUG=y
+CONFIG_CMD_TIME=y
+CONFIG_SPL_OF_CONTROL=y
+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_MMC=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_DFU_MMC=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_MISC=y
+CONFIG_ROCKCHIP_EFUSE=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_ROCKCHIP=y
+CONFIG_ETH_DESIGNWARE=y
+CONFIG_GMAC_ROCKCHIP=y
+CONFIG_NVME_PCI=y
+CONFIG_PCI=y
+CONFIG_PHY_ROCKCHIP_INNO_USB2=y
+CONFIG_PHY_ROCKCHIP_TYPEC=y
+CONFIG_PMIC_RK8XX=y
+CONFIG_REGULATOR_PWM=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM_ROCKCHIP_LPDDR4=y
+CONFIG_BAUDRATE=150
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550_MEM32=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_DWC3=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_USB_HOST_ETHER=y
+CONFIG_USB_ETHER_ASIX=y
+CONFIG_USB_ETHER_ASIX88179=y
+CONFIG_USB_ETHER_MCS7830=y
+CONFIG_USB_ETHER_RTL8152=y
+CONFIG_USB_ETHER_SMSC95XX=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_FUNCTION_ROCKUSB=y
+CONFIG_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+CONFIG_SPL_TINY_MEMSET=y
+CONFIG_ERRNO_STR=y
+CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
-- 
2.39.2



[PATCH 2/5] configs: rockchip: rock-pi-4: use dtb for ROCK Pi 4A instead of 4B

2023-04-25 Thread FUKAUMI Naoki
rk3399-rock-pi-4a.dtb is enough for Radxa ROCK Pi 4A/B/A+/B+ and ROCK 4SE.

Signed-off-by: FUKAUMI Naoki 
---
 configs/rock-pi-4-rk3399_defconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/rock-pi-4-rk3399_defconfig 
b/configs/rock-pi-4-rk3399_defconfig
index 4ecc06d92b..fde1ee2eb5 100644
--- a/configs/rock-pi-4-rk3399_defconfig
+++ b/configs/rock-pi-4-rk3399_defconfig
@@ -7,7 +7,7 @@ CONFIG_NR_DRAM_BANKS=1
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x30
 CONFIG_ENV_OFFSET=0x3F8000
-CONFIG_DEFAULT_DEVICE_TREE="rk3399-rock-pi-4b"
+CONFIG_DEFAULT_DEVICE_TREE="rk3399-rock-pi-4a"
 CONFIG_DM_RESET=y
 CONFIG_ROCKCHIP_RK3399=y
 CONFIG_TARGET_EVB_RK3399=y
@@ -17,7 +17,7 @@ CONFIG_DEBUG_UART_CLOCK=2400
 CONFIG_SYS_LOAD_ADDR=0x800800
 CONFIG_DEBUG_UART=y
 # CONFIG_ANDROID_BOOT_IMAGE is not set
-CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4b.dtb"
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4a.dtb"
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_MISC_INIT_R=y
 CONFIG_SPL_MAX_SIZE=0x2e000
-- 
2.39.2



[PATCH 5/5] doc: rockchip: update list of Radxa ROCK (Pi) 4 boards

2023-04-25 Thread FUKAUMI Naoki
add Radxa ROCK (Pi) 4 variants.

Signed-off-by: FUKAUMI Naoki 
---
 doc/board/rockchip/rockchip.rst | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/board/rockchip/rockchip.rst b/doc/board/rockchip/rockchip.rst
index 1dccb17d72..749e40de01 100644
--- a/doc/board/rockchip/rockchip.rst
+++ b/doc/board/rockchip/rockchip.rst
@@ -83,7 +83,10 @@ List of mainline supported Rockchip boards:
  - Khadas Edge-V (hadas-edge-v-rk3399)
  - Orange Pi RK3399 (orangepi-rk3399)
  - Pine64 RockPro64 (rockpro64-rk3399)
- - Radxa ROCK Pi 4 (rock-pi-4-rk3399)
+ - Radxa ROCK 4C+ (rock-4c-plus-rk3399)
+ - Radxa ROCK 4SE (rock-pi-4-rk3399)
+ - Radxa ROCK Pi 4A/B/A+/B+ (rock-pi-4-rk3399)
+ - Radxa ROCK Pi 4C (rock-pi-4c-rk3399)
  - Rockchip Evb-RK3399 (evb_rk3399)
  - Theobroma Systems RK3399-Q7 SoM - Puma (puma_rk3399)
 
-- 
2.39.2



Re: [PATCH V2 01/53] spl: imx8mm: enlarge SPL_MAX_SIZE

2023-04-25 Thread Peng Fan




On 4/20/2023 2:57 PM, Frieder Schrempf wrote:

Hi,

On 26.07.22 10:40, Peng Fan (OSS) wrote:

From: Peng Fan 

The CONFIG_SPL_MAX_SIZE could be 0x27000 for i.MX8MM when SPL_TEXT_BASE
set to 0x7E1000.

The DDR firmware max uses 96KB, there is a 4KB padding header before
SPL_TEXT_BASE, so the SPL MAX SIZE is `256KB - 96KB - 4KB`.


Am I right to assume that setting CONFIG_SPL_MAX_SIZE to 0x27000 is more
restricting nowadays than it would need to be?

If I understand correctly nowadays the DDR firmware doesn't need a fixed
96 KiB area with padding anymore, but rather takes up only the space it
needs (~ 57 KiB) due to "binman magic".

If this is correct, is it safe to increase CONFIG_SPL_MAX_SIZE?
Is there a sane default that we could use instead of 0x27000?


With binman to pack images, it is true that the ddr firmware is not that
large. The CONFIG_SPL_MAX_SIZE could be enlarged.

I am not sure what default value could be used.

Regards,
Peng.



Thanks
Frieder


Re: [PATCH] imx: fix get_boot_device() for imx8

2023-04-25 Thread Peng Fan




On 4/25/2023 12:33 AM, Tim Harvey wrote:

commit 787f04bb6a0a ("imx: add USB2_BOOT type") broke get_boot_device()
for IMX8 which affects booting from SDP due to boot_instance being
non-zero.

Fix this by only using boot_instance for imx8ulp and imx9.

Fixes: 787f04bb6a0a ("imx: add USB2_BOOT type")
Signed-off-by: Tim Harvey 


Reviewed-by: Peng Fan 


---
  arch/arm/mach-imx/romapi.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-imx/romapi.c b/arch/arm/mach-imx/romapi.c
index b49e7f80a286..ff0522c2d117 100644
--- a/arch/arm/mach-imx/romapi.c
+++ b/arch/arm/mach-imx/romapi.c
@@ -70,6 +70,8 @@ enum boot_device get_boot_device(void)
boot_dev = SPI_NOR_BOOT;
break;
case BT_DEV_TYPE_USB:
+   if (!is_imx8ulp() && !is_imx9())
+   boot_instance = 0;
boot_dev = boot_instance + USB_BOOT;
break;
default:


Re: [PATCH] arm: imx8m: remove unused and obsolete board_fix_fdt() in SOC context

2023-04-25 Thread Peng Fan




On 4/25/2023 10:19 PM, Hugo Villeneuve wrote:

From: Hugo Villeneuve 

It doesn't seem appropriate for arch/SOC to use a board-level
functionality (CONFIG_OF_BOARD_FIXUP), because this prevents boards
that need to do FDT fixup from using that feature.

Also, this code is completely dead and useless (from comments by
Rasmus Villemoes on the mailing list):

   - No in-tree imx8m-based board seems to set CONFIG_OF_BOARD_FIXUP
   - The nodes which that function wants to disable don't even exist in
 the U-Boot copy of imx8mp.dtsi.

This code was introduced in commit 35bb60787b88. It seems to be some
random import of code from downstream NXP U-Boot, with a commit
message that makes no sense in upstream context.

Signed-off-by: Hugo Villeneuve 


Acked-by: Peng Fan 

---
  arch/arm/mach-imx/imx8m/soc.c | 34 --
  1 file changed, 34 deletions(-)

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index df865e997d..2ec5c3dc43 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -1395,40 +1395,6 @@ usb_modify_speed:
  }
  #endif
  
-#ifdef CONFIG_OF_BOARD_FIXUP

-#ifndef CONFIG_SPL_BUILD
-int board_fix_fdt(void *fdt)
-{
-   if (is_imx8mpul()) {
-   int i = 0;
-   int nodeoff, ret;
-   const char *status = "disabled";
-   static const char * const dsi_nodes[] = {
-   "/soc@0/bus@32c0/mipi_dsi@32e6",
-   "/soc@0/bus@32c0/lcd-controller@32e8",
-   "/dsi-host"
-   };
-
-   for (i = 0; i < ARRAY_SIZE(dsi_nodes); i++) {
-   nodeoff = fdt_path_offset(fdt, dsi_nodes[i]);
-   if (nodeoff > 0) {
-set_status:
-   ret = fdt_setprop(fdt, nodeoff, "status", 
status,
- strlen(status) + 1);
-   if (ret == -FDT_ERR_NOSPACE) {
-   ret = fdt_increase_size(fdt, 512);
-   if (!ret)
-   goto set_status;
-   }
-   }
-   }
-   }
-
-   return 0;
-}
-#endif
-#endif
-
  #if !CONFIG_IS_ENABLED(SYSRESET)
  void reset_cpu(void)
  {


Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

2023-04-25 Thread Simon Glass
Hi Filip,

On Tue, 25 Apr 2023 at 06:36, Filip Žaludek  wrote:
>
>
>
> Hi Simon,
>
>
> On 4/19/23 03:49, Simon Glass wrote:
> > Hi Filip,
> >
> > On Tue, 11 Apr 2023 at 14:24, Filip Žaludek  
> > wrote:
> >>
> >>
> >>
> >> On 2/8/23 20:01, Mark Kettenis wrote:
>  Date: Wed, 8 Feb 2023 19:45:36 +0100
>  From: Michal Suchánek 
> 
>  Hello,
> 
>  On Wed, Jan 18, 2023 at 05:01:12PM +0100, Filip Žaludek wrote:
> >
> >
> > Hi Michal,
> >
> >thanks for testing! Do you consider keyboard as working once it is 
> > detected without
> > 'usb_kbd usb_kbd: Timeout poll on interrupt endpoint', or judging from 
> > subsequent
> > typing? Note that issue is reproducible only in about 20% of reboots.
> 
>  I rely on keyboard input to boot so if it was 20% broken I would notice.
>  I don't use the rPi all that much so if it was broken only a few
>  % of the time there is a chance I would miss it.
> 
>  However, for me not typing on the keyboard during usb detection it is
>  100% not detected, typing on it during usb detection it is 100%
>  detected.
> 
>  The timeout is limitation of the dwc2 controller handling of usb hubs.
> 
>  There might be a possibility to improve the driver so that it handles
>  the condition but it might be that the Linux driver relies on a separate
>  thread handling the controller which is not acceptable for u-boot.
> 
>  I am not usb expert and definitely not dwc2 expert so I cannot do more
>  than workaround the current driver limitation.
> 
> > For me I can always enter 'U-Boot>' shell, but then keyboard usually 
> > does not work.
> > And yes, resetting the usb controller with pressing a key afterwards 
> > will
> > finally break the keyboard. ('usb reset' typed from keyboard)
> > If you are Prague located I am ready to demonstrate what I am talking 
> > about.
> >
> >Simon's keyboard detection is somewhat interfered by 'SanDisk USB 
> > Extreme Pro' detection,
> > printed complaints but keyboard still works..
> > 'usb_kbd usb_kbd: Timeout poll on interrupt endpoint' and 'Failed to 
> > get keyboard state from device 0c40:8000'
> > Btw. why from 0c40:8000 (ELMCU 2.4GHz receiver) when wired keyboard is 
> > 046d:c31c (Logitech Keyboard K120)?
> >
> >What is supposed scenario for RPi3/u-boot/grub usb keyboard equipped 
> > users wanting to boot non-default?
> > Enter 'U-Boot>' shell to detect keyboard; type boot; select desired 
> > grub entry..?
> >
> > Reverting either from the two makes it non issue for me:
> > 'dwc2: use the nonblock argument in submit_int_msg'
> > commit 9dcab2c4d2cb50ab1864c818b82a72393c160236
> 
>  Without this booting from USB is not feasible because reading every
>  block from the USB drive waits for the keyboard to time out.
> 
> > 'console: usb: kbd: Limit poll frequency to improve performance'
> > commit 96991e652f541323a03c5b7e075d54a117091618
> 
>  No idea about this one, for me it doea not give any substantial
>  difference in behavior.
> >>>
> >>> Reverting that commit leads to a significant slowdown loading a kernel
> >>> from disk with a usb keyboard connected.  The slowdown is somewhat
> >>> hardware dependent but on some systems loading the OpenBSD/arm64
> >>> kernel would take minutes instead of seconds.
> >>
> >>
> >> Hello,
> >> I am about to dig more into this issue with proper tools, but failed to
> >> configure/compile trace functionality on RPi3 due to missing references
> >> to timer_early_get_count() and timer_early_get_rate().
> >
> > You could implement a proper timer driver for rpi.
> >
> >>
> >>Is it possible/feasible to implement calls in CONFIG_SYS_ARCH_TIMER
> >> and/or CONFIG_SP804_TIMER?
> >
> > Yes
>
>
>   I am little bit missing here secret sauce, timer_early_get_count() and 
> timer_early_get_rate()
> are not supposed to be implemented in arch/arm/cpu/armv8/generic_timer.c? But 
> predestined for
> drivers/timer/sp804_timer.c?
>
> TIMER is required for common/board_f.c and common/board_r.c but it disables 
> generic_timer..
> %< -
> ifndef CONFIG_$(SPL_TPL_)TIMER
> obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o
> endif
> %< -
> And obviously multiple definition of get_tbclk and get_ticks when forced to 
> compile/link.

It seems from the above that if TIMER is enabled for a particular
U-Boot build phase, then generic_timer is not, and vice versa. I
suppose that is fair enough.

>
>
>
>
> >>
> >>I would be grateful even for trace to generate function traces without
> >> timestamps. Is such nasty hack without timestamping supposed to work?
> >> Basically my intention is to trace 'usb reset'.
> >>
> >> Appreciate any hints/outlines how to proceed.
> >
> > I assume you mean 

Re: [PATCH v2] common/Kconfig: fix comments syntax error

2023-04-25 Thread Simon Glass
On Tue, 25 Apr 2023 at 12:34, Hugo Villeneuve  wrote:
>
> From: Hugo Villeneuve 
>
> Fix comments error in EVENT_DEBUG description:
> this get usefui -> this to get useful
>
> Signed-off-by: Hugo Villeneuve 
> ---
>  common/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

2023-04-25 Thread Simon Glass
Hi Pali,

On Tue, 25 Apr 2023 at 10:28, Pali Rohár  wrote:
>
> On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> > This creates a lot of errors of the form:
> >
> > `__stack_chk_fail' referenced in section `.text' of ...ltrans.o: defined
> >in discarded section `.text' of common/stackprot.o (symbol from plugin)
>
> This issue should be rather fixed...
>
> > Drop LTO for now.
>
> ... and until it happens is not CONFIG_LTO for disabling enough?
>
> LTO does not work for more other boards / platforms and it is just _not_
> enabled via CONFIG_LTO in those cases...

The thing is, LTO is enabled for sandbox normally (clang and gcc). It
is just the MSYS2 platform where there are problems.

I am not sure how to fix it, since I don't know what discarded section
it is referring to.

Regards,
Simon


Re: distro_boot vs. env-based bootmenu

2023-04-25 Thread Simon Glass
+Heinrich Schuchardt +Ilias Apalodimas in case they have ideas on this

On Mon, 24 Apr 2023 at 15:22, Frank Wunderlich  wrote:
>
> Am 24. April 2023 21:44:25 MESZ schrieb Simon Glass :
> >Hi Frank,
> >
> >On Fri, 21 Apr 2023 at 10:03, Frank Wunderlich  
> >wrote:
> >>
> >> Maybe you have an idea because your patch removes distro-boot
> >>
> >> https://patchwork.ozlabs.org/project/uboot/patch/20230409084454.v9.8.I4cf7708a1ba953b9abd81375d93af34665c7b251@changeid/
> >>
> >> For missing linebreak for autoboot this seems to be a calculation issue
> >>
> >> https://source.denx.de/u-boot/u-boot/-/blob/master/common/menu.c#L439
> >
> >Are you able to fix it with a patch?
>
> Have not yet figured out which part of u-boot creates the mmc-entries on top 
> of my bootmenu (i guess this comes from my still enabled distroboot options).
>
> For missing linebreak i guess position calculation is wrong in this case.
>
> >Or if not, do you have a patch for the 'move to env-based bootmenu' so
> >I can try it?
>
> I have i builtin env file (uEnv_r2pro.txt) in my tree here and add (enable 
> pepared option) the config-option to use this env file in topmost commit.
>
> https://github.com/frank-w/u-boot/commits/2023-04-bpi-r2pro
>
> I had this in but disabled but this overrides the env created by 
> distroboot-defines...but now enabling it to get rid of distroboot.
>
> >Regards,
> >Simon
> >
> >
> >
> >>
> >>
> >>
> >>  Ursprüngliche Nachricht 
> >> Von: Frank Wunderlich 
> >> Gesendet: 12. April 2023 11:22:23 MESZ
> >> An: u-boot@lists.denx.de
> >> Betreff: distro_boot vs. env-based bootmenu
> >>
> >> Hi,
> >>
> >> i try to move from distro-boot (extlinux-config files) to an env-based 
> >> bootmenu (builtin-environment) for
> >> my bananapi r2pro.
> >>
> >> basicly it works, but i see more bootmenu-entries than i have defined in 
> >> my environment
> >>
> >>  *** U-Boot Boot Menu ***
> >>
> >>   1. Boot from SD/EMMC.
> >>   mmc 1:2
> >>   mmc 1:3
> >>   mmc 1:4
> >>   U-Boot consoleHit any key to stop autoboot: 2
> >>
> >> i have only defined one botmenu-entry (first one in uEnv_r2pro.txt)
> >>
> >> bootmenu_0=1. Boot from SD/EMMC.=run newboot
> >> bootmenu_default=0
> >>
> >> i guess the 3 mmc 1:x entries are part from distro-boot. i see a series 
> >> from simon where he drops
> >> distroboot for rockchip-boards.
> >>
> >> https://patchwork.ozlabs.org/project/uboot/patch/20230409084454.v9.8.I4cf7708a1ba953b9abd81375d93af34665c7b251@changeid/
> >>
> >> can anyone explain me where i can disable the mmc 1:x entries? i wanted to 
> >> leave distro_boot as
> >> fallback but it seems this is not possible.
> >>
> >> maybe i have to disable these?
> >>
> >> BOOTMETH_DISTRO [=y]
> >> DISTRO_DEFAULTS [=y]
> >>
> >> can i keep distro-boot (seems to works with env too, but overridden by my 
> >> builtin env) somehow and drop the mmc-entries from my bootmenu?
> >>
> >> any idea why i have no linebreak after u-Boot console?
> >>
> >> i use u-boot 2023.04 my source for this conversion/tests is there:
> >> https://github.com/frank-w/u-boot/tree/2023-04-bpi-r2pro
> >> using modified configs/evb-rk3568_defconfig
> >>
> >> regards Frank
> >> regards Frank
>
>
> regards Frank


Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

2023-04-25 Thread Simon Glass
Hi Pali,

On Tue, 25 Apr 2023 at 13:33, Pali Rohár  wrote:
>
> On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> > Hi Pali,
> >
> > On Tue, 25 Apr 2023 at 12:11, Pali Rohár  wrote:
> > >
> > > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > > Hi Pali,
> > > >
> > > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár  wrote:
> > > > >
> > > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > > >
> > > > > > Signed-off-by: Simon Glass 
> > > > > > ---
> > > > > >
> > > > > >  Makefile   |  11 +-
> > > > > >  arch/sandbox/config.mk |   4 +
> > > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 
> > > > > > +
> > > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > > >
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -1730,6 +1730,13 @@ else
> > > > > >  u-boot-keep-syms-lto :=
> > > > > >  endif
> > > > > >
> > > > > > +ifeq ($(MSYS_VERSION),0)
> > > > > > +add_ld_script := -T u-boot.lds
> > > > > > +else
> > > > > > +add_ld_script := u-boot.lds
> > > > > > +$(warning msys)
> > > > > > +endif
> > > > > > +
> > > > > >  # Rule to link u-boot
> > > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > > @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > > > > >   $(CC) -nostdlib -nostartfiles 
> > > > > >   \
> > > > > >   $(LTO_FINAL_LDFLAGS) $(c_flags)   
> > > > > >   \
> > > > > >   $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) 
> > > > > > -o $@   \
> > > > > > - -T u-boot.lds $(u-boot-init)  
> > > > > >   \
> > > > > > + $(add_ld_script) $(u-boot-init)   
> > > > > >   \
> > > > > >   -Wl,--whole-archive   
> > > > > >   \
> > > > > >   $(u-boot-main)
> > > > > >   \
> > > > > >   $(u-boot-keep-syms-lto)   
> > > > > >   \
> > > > > > @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > > > > >  else
> > > > > >  quiet_cmd_u-boot__ ?= LD  $@
> > > > > >cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o 
> > > > > > $@\
> > > > > > - -T u-boot.lds $(u-boot-init)  
> > > > > >   \
> > > > > > + $(add_ld_script) $(u-boot-init)   
> > > > > >   \
> > > > > >   --whole-archive   
> > > > > >   \
> > > > > >   $(u-boot-main)
> > > > > >   \
> > > > > >   --no-whole-archive
> > > > > >   \
> > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > > --- a/arch/sandbox/config.mk
> > > > > > +++ b/arch/sandbox/config.mk
> > > > > > @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
> > > > > >  EFI_RELOC := reloc_sandbox_efi.o
> > > > > >  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > > >  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > > > +
> > > > > > +ifneq ($(MSYS_VERSION),0)
> > > > > > +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > +endif
> > > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds 
> > > > > > b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > new file mode 100644
> > > > > > index ..031e70fafd03
> > > > > > --- /dev/null
> > > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > @@ -0,0 +1,447 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > +/*
> > > > > > + * U-Boot note: This was obtained by using the -verbose linker 
> > > > > > option. The
> > > > > > + * U-Boot additions are marked below.
> > > > > > + *
> > > > > > + * Ideally we would add sections to the executable, as is done 
> > > > > > with the Linux
> > > > > > + * build. But PE executables do not appear to work correctly if 
> > > > > > unexpected
> > > > > > + * sections are present:
> > > > > > + *
> > > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: 
> > > > > > Exec format error
> > > > > > + *
> > > > > > + * So we take a approach of rewriting the whole file, for now. 
> > > > > > This will likely
> > > > > > + * break in the future when a toolchain change is made.
> > > > >
> > > > > Why 

Re: [PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi

2023-04-25 Thread Tom Rini
On Tue, Apr 25, 2023 at 10:02:01PM +0200, Rasmus Villemoes wrote:
> On 25/04/2023 21.31, Tom Rini wrote:
> > On Fri, Mar 17, 2023 at 11:26:39AM +0100, Rasmus Villemoes wrote:
> > 
> 
> >> Now, the only way to be really sure is to build the world
> >> with/without this patch and check if any .dtb file changes, but I
> >> don't have the means to do that.
> > 
> > So, yes, this causes a bunch of fail to builds, as you noted above. The
> > easiest way I think to confirm things before / after would be to make a
> > quick change to tools/buildman/builderthread.py and self.CopyFiles line
> > for keep_outputs to also keep the dtb or some dts files so you can diff
> > before / after to make sure the end result is the same.
> 
> Do the builds outright fail, or do they fail in the sense that some
> machinery detects a change in the binary artifacts? Can you point me at
> one or two CI builds that show this?

They outright fail to build, mt8516_pumpkin is the one I was testing
with.  CI didn't get to the point of failing on that particular problem
(as it's in the last stage and qemu randomly failed as it does earlier).

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi

2023-04-25 Thread Rasmus Villemoes
On 25/04/2023 21.31, Tom Rini wrote:
> On Fri, Mar 17, 2023 at 11:26:39AM +0100, Rasmus Villemoes wrote:
> 

>> Now, the only way to be really sure is to build the world
>> with/without this patch and check if any .dtb file changes, but I
>> don't have the means to do that.
> 
> So, yes, this causes a bunch of fail to builds, as you noted above. The
> easiest way I think to confirm things before / after would be to make a
> quick change to tools/buildman/builderthread.py and self.CopyFiles line
> for keep_outputs to also keep the dtb or some dts files so you can diff
> before / after to make sure the end result is the same.

Do the builds outright fail, or do they fail in the sense that some
machinery detects a change in the binary artifacts? Can you point me at
one or two CI builds that show this?

Rasmus





Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

2023-04-25 Thread Eugen Hristev

On 4/25/23 22:22, Ramon Fried wrote:

On Tue, Apr 25, 2023 at 4:17 PM Eugen Hristev
 wrote:


On 4/25/23 16:06, Eugen Hristev wrote:

Add minimal support for 8125B version.
Changes are based on the Linux driver.
Tested on Radxa Rock 5B Rk3588 board.

Connection to a laptop worked fine in 100 Mbps mode.
1000 Mbps mode is not working at the moment.

Signed-off-by: Eugen Hristev 
---


The one thing that impacts all the rtl chips is the way the pci BAR is
now mapped.
I could not test this on another platform so help on this matter is
appreciated.

Thanks!
Eugen

Let's wait a bit to see if someone can test it. why did you change the
mapping of the BAR ?


It did not work with the old code. It provided a bad address, to some 
area which did not have the right registers.
I looked into similar drivers and they were using this call, which works 
perfectly for 8125b device


Eugen


Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

2023-04-25 Thread Pali Rohár
On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 12:11, Pali Rohár  wrote:
> >
> > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár  wrote:
> > > >
> > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > >
> > > > > Signed-off-by: Simon Glass 
> > > > > ---
> > > > >
> > > > >  Makefile   |  11 +-
> > > > >  arch/sandbox/config.mk |   4 +
> > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 
> > > > > +
> > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -1730,6 +1730,13 @@ else
> > > > >  u-boot-keep-syms-lto :=
> > > > >  endif
> > > > >
> > > > > +ifeq ($(MSYS_VERSION),0)
> > > > > +add_ld_script := -T u-boot.lds
> > > > > +else
> > > > > +add_ld_script := u-boot.lds
> > > > > +$(warning msys)
> > > > > +endif
> > > > > +
> > > > >  # Rule to link u-boot
> > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > > > >   $(CC) -nostdlib -nostartfiles   
> > > > > \
> > > > >   $(LTO_FINAL_LDFLAGS) $(c_flags) 
> > > > > \
> > > > >   $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o 
> > > > > $@   \
> > > > > - -T u-boot.lds $(u-boot-init)
> > > > > \
> > > > > + $(add_ld_script) $(u-boot-init) 
> > > > > \
> > > > >   -Wl,--whole-archive 
> > > > > \
> > > > >   $(u-boot-main)  
> > > > > \
> > > > >   $(u-boot-keep-syms-lto) 
> > > > > \
> > > > > @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > > > >  else
> > > > >  quiet_cmd_u-boot__ ?= LD  $@
> > > > >cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o 
> > > > > $@\
> > > > > - -T u-boot.lds $(u-boot-init)
> > > > > \
> > > > > + $(add_ld_script) $(u-boot-init) 
> > > > > \
> > > > >   --whole-archive 
> > > > > \
> > > > >   $(u-boot-main)  
> > > > > \
> > > > >   --no-whole-archive  
> > > > > \
> > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > --- a/arch/sandbox/config.mk
> > > > > +++ b/arch/sandbox/config.mk
> > > > > @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
> > > > >  EFI_RELOC := reloc_sandbox_efi.o
> > > > >  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > >  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > > +
> > > > > +ifneq ($(MSYS_VERSION),0)
> > > > > +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > +endif
> > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds 
> > > > > b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > new file mode 100644
> > > > > index ..031e70fafd03
> > > > > --- /dev/null
> > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > @@ -0,0 +1,447 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +/*
> > > > > + * U-Boot note: This was obtained by using the -verbose linker 
> > > > > option. The
> > > > > + * U-Boot additions are marked below.
> > > > > + *
> > > > > + * Ideally we would add sections to the executable, as is done with 
> > > > > the Linux
> > > > > + * build. But PE executables do not appear to work correctly if 
> > > > > unexpected
> > > > > + * sections are present:
> > > > > + *
> > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: 
> > > > > Exec format error
> > > > > + *
> > > > > + * So we take a approach of rewriting the whole file, for now. This 
> > > > > will likely
> > > > > + * break in the future when a toolchain change is made.
> > > >
> > > > Why not rather provide "layer" linker script which does this "rewriting"
> > > > on top of the default linker script? With this way it is not needed to
> > > > update linker script when a toolchain change it.
> > > >
> > >
> > > How can we reliably do that, though? We don't really know the 

Re: [PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi

2023-04-25 Thread Tom Rini
On Fri, Mar 17, 2023 at 11:26:39AM +0100, Rasmus Villemoes wrote:

> I have a couple of boards, e.g. foo21, bar33, each with a few
> different revisions, so I have
> 
>   foo21-revA.dts
>   foo21-revB.dts
>   bar33-revA.dts
>   bar33-revB.dts
> 
> Now the necessary U-Boot specific additions for the foo21 boards
> doesn't depend on the revision (likely for bar33), so I just want to
> have and maintain one
> 
>   foo21-u-boot.dtsi
> 
> But currently I need to add dummy files foo21-revA-u-boot.dtsi and
> foo21-revB-u-boot.dtsi each just containing '#include
> foo21-u-boot.dtsi'. And similarly for bar33, and all those files
> become quite unwieldy as more revisions need to be supported.
> 
> It's quite natural to look for a file named after CONFIG_SYS_BOARD,
> with lower precedence of course than a -u-boot.dtsi file with the same
> basename as the .dts, but higher than CONFIG_SYS_SOC.
> 
> Signed-off-by: Rasmus Villemoes 
> Reviewed-by: Simon Glass 
> Reviewed-by: Simon Glass 
> ---
> 
> Of course, this can cause unwanted changes for existing boards. But a
> bit of ad hoc scripting shows that the risk is low: I first grabbed
> all 'default "foo"' stanzas of all 'config SYS_BOARD' instances, as
> well as all values of CONFIG_SYS_BOARD set in *_defconfig files. Then
> I made a list of all existing *-u-boot.dtsi files, and from these
> removed any where there is a corresponding .dts file. That leaves just
> 
> imx6ul
> imx8mm
> mt7620
> mt7621
> mt7622
> mt7623
> mt7628
> mt8516
> socfpga_arria10
> sunxi
> 
> and inspecting a few of those suggests that they set SYS_SOC to the
> same as SYS_BOARD, i.e. they were already picking up the .dtsi file
> due to the SYS_SOC rule.
> 
> Now, the only way to be really sure is to build the world
> with/without this patch and check if any .dtb file changes, but I
> don't have the means to do that. But I do notice that 

So, yes, this causes a bunch of fail to builds, as you noted above. The
easiest way I think to confirm things before / after would be to make a
quick change to tools/buildman/builderthread.py and self.CopyFiles line
for keep_outputs to also keep the dtb or some dts files so you can diff
before / after to make sure the end result is the same.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH] arm: dts: iot2050: Include u-boot specific bits implicitly

2023-04-25 Thread Jan Kiszka
From: Jan Kiszka 

Create *-u-boot.dtsi files for each target dtb of the IOT2050 series so
that we can drop the #include deviations from upstream dts[i] files
here.

Signed-off-by: Jan Kiszka 
---

This was tested against most of our boards with the DTS synced from 
kernel 6.3.

 arch/arm/dts/k3-am65-iot2050-common-pg2.dtsi  |  2 --
 arch/arm/dts/k3-am6528-iot2050-basic-common.dtsi  |  3 ---
 arch/arm/dts/k3-am6528-iot2050-basic-pg2-u-boot.dtsi  | 11 +++
 arch/arm/dts/k3-am6528-iot2050-basic-u-boot.dtsi  | 10 ++
 arch/arm/dts/k3-am6548-iot2050-advanced-common.dtsi   |  3 ---
 .../arm/dts/k3-am6548-iot2050-advanced-m2-u-boot.dtsi |  1 +
 .../dts/k3-am6548-iot2050-advanced-pg2-u-boot.dtsi|  1 +
 arch/arm/dts/k3-am6548-iot2050-advanced-u-boot.dtsi   |  1 +
 8 files changed, 24 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/dts/k3-am6528-iot2050-basic-pg2-u-boot.dtsi
 create mode 100644 arch/arm/dts/k3-am6528-iot2050-basic-u-boot.dtsi
 create mode 12 arch/arm/dts/k3-am6548-iot2050-advanced-m2-u-boot.dtsi
 create mode 12 arch/arm/dts/k3-am6548-iot2050-advanced-pg2-u-boot.dtsi
 create mode 12 arch/arm/dts/k3-am6548-iot2050-advanced-u-boot.dtsi

diff --git a/arch/arm/dts/k3-am65-iot2050-common-pg2.dtsi 
b/arch/arm/dts/k3-am65-iot2050-common-pg2.dtsi
index e7e0ca41597..e73458ca690 100644
--- a/arch/arm/dts/k3-am65-iot2050-common-pg2.dtsi
+++ b/arch/arm/dts/k3-am65-iot2050-common-pg2.dtsi
@@ -49,5 +49,3 @@
snps,dis-u1-entry-quirk;
snps,dis-u2-entry-quirk;
 };
-
-#include "k3-am65-iot2050-common-pg2-u-boot.dtsi"
diff --git a/arch/arm/dts/k3-am6528-iot2050-basic-common.dtsi 
b/arch/arm/dts/k3-am6528-iot2050-basic-common.dtsi
index 0d215b4d668..4a9bf7d7c07 100644
--- a/arch/arm/dts/k3-am6528-iot2050-basic-common.dtsi
+++ b/arch/arm/dts/k3-am6528-iot2050-basic-common.dtsi
@@ -11,9 +11,6 @@
 
 #include "k3-am65-iot2050-common.dtsi"
 
-#include "k3-am65-iot2050-common-u-boot.dtsi"
-#include "k3-am65-iot2050-boot-image.dtsi"
-
 / {
memory@8000 {
device_type = "memory";
diff --git a/arch/arm/dts/k3-am6528-iot2050-basic-pg2-u-boot.dtsi 
b/arch/arm/dts/k3-am6528-iot2050-basic-pg2-u-boot.dtsi
new file mode 100644
index 000..1e393042ac0
--- /dev/null
+++ b/arch/arm/dts/k3-am6528-iot2050-basic-pg2-u-boot.dtsi
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2023
+ *
+ * Authors:
+ *   Jan Kiszka 
+ */
+
+#include "k3-am65-iot2050-common-u-boot.dtsi"
+#include "k3-am65-iot2050-common-pg2-u-boot.dtsi"
+#include "k3-am65-iot2050-boot-image.dtsi"
diff --git a/arch/arm/dts/k3-am6528-iot2050-basic-u-boot.dtsi 
b/arch/arm/dts/k3-am6528-iot2050-basic-u-boot.dtsi
new file mode 100644
index 000..64afe25e38f
--- /dev/null
+++ b/arch/arm/dts/k3-am6528-iot2050-basic-u-boot.dtsi
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2023
+ *
+ * Authors:
+ *   Jan Kiszka 
+ */
+
+#include "k3-am65-iot2050-common-u-boot.dtsi"
+#include "k3-am65-iot2050-boot-image.dtsi"
diff --git a/arch/arm/dts/k3-am6548-iot2050-advanced-common.dtsi 
b/arch/arm/dts/k3-am6548-iot2050-advanced-common.dtsi
index 816a4cb4a68..d25e8b26187 100644
--- a/arch/arm/dts/k3-am6548-iot2050-advanced-common.dtsi
+++ b/arch/arm/dts/k3-am6548-iot2050-advanced-common.dtsi
@@ -13,9 +13,6 @@
 
 #include "k3-am65-iot2050-common.dtsi"
 
-#include "k3-am65-iot2050-common-u-boot.dtsi"
-#include "k3-am65-iot2050-boot-image.dtsi"
-
 / {
memory@8000 {
device_type = "memory";
diff --git a/arch/arm/dts/k3-am6548-iot2050-advanced-m2-u-boot.dtsi 
b/arch/arm/dts/k3-am6548-iot2050-advanced-m2-u-boot.dtsi
new file mode 12
index 000..859776d3ffe
--- /dev/null
+++ b/arch/arm/dts/k3-am6548-iot2050-advanced-m2-u-boot.dtsi
@@ -0,0 +1 @@
+k3-am6528-iot2050-basic-pg2-u-boot.dtsi
\ No newline at end of file
diff --git a/arch/arm/dts/k3-am6548-iot2050-advanced-pg2-u-boot.dtsi 
b/arch/arm/dts/k3-am6548-iot2050-advanced-pg2-u-boot.dtsi
new file mode 12
index 000..859776d3ffe
--- /dev/null
+++ b/arch/arm/dts/k3-am6548-iot2050-advanced-pg2-u-boot.dtsi
@@ -0,0 +1 @@
+k3-am6528-iot2050-basic-pg2-u-boot.dtsi
\ No newline at end of file
diff --git a/arch/arm/dts/k3-am6548-iot2050-advanced-u-boot.dtsi 
b/arch/arm/dts/k3-am6548-iot2050-advanced-u-boot.dtsi
new file mode 12
index 000..ac30e4ef46e
--- /dev/null
+++ b/arch/arm/dts/k3-am6548-iot2050-advanced-u-boot.dtsi
@@ -0,0 +1 @@
+k3-am6528-iot2050-basic-u-boot.dtsi
\ No newline at end of file
-- 
2.35.3


Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

2023-04-25 Thread Simon Glass
Hi Pali,

On Tue, 25 Apr 2023 at 12:11, Pali Rohár  wrote:
>
> On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > Hi Pali,
> >
> > On Tue, 25 Apr 2023 at 10:21, Pali Rohár  wrote:
> > >
> > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > Windows. Add a note as to why this seems to be necessary for now.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > >  Makefile   |  11 +-
> > > >  arch/sandbox/config.mk |   4 +
> > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +
> > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1730,6 +1730,13 @@ else
> > > >  u-boot-keep-syms-lto :=
> > > >  endif
> > > >
> > > > +ifeq ($(MSYS_VERSION),0)
> > > > +add_ld_script := -T u-boot.lds
> > > > +else
> > > > +add_ld_script := u-boot.lds
> > > > +$(warning msys)
> > > > +endif
> > > > +
> > > >  # Rule to link u-boot
> > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > >  ifeq ($(LTO_ENABLE),y)
> > > > @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > > >   $(CC) -nostdlib -nostartfiles 
> > > >   \
> > > >   $(LTO_FINAL_LDFLAGS) $(c_flags)   
> > > >   \
> > > >   $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@ 
> > > >   \
> > > > - -T u-boot.lds $(u-boot-init)  
> > > >   \
> > > > + $(add_ld_script) $(u-boot-init)   
> > > >   \
> > > >   -Wl,--whole-archive   
> > > >   \
> > > >   $(u-boot-main)
> > > >   \
> > > >   $(u-boot-keep-syms-lto)   
> > > >   \
> > > > @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > > >  else
> > > >  quiet_cmd_u-boot__ ?= LD  $@
> > > >cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@  
> > > >   \
> > > > - -T u-boot.lds $(u-boot-init)  
> > > >   \
> > > > + $(add_ld_script) $(u-boot-init)   
> > > >   \
> > > >   --whole-archive   
> > > >   \
> > > >   $(u-boot-main)
> > > >   \
> > > >   --no-whole-archive
> > > >   \
> > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > --- a/arch/sandbox/config.mk
> > > > +++ b/arch/sandbox/config.mk
> > > > @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
> > > >  EFI_RELOC := reloc_sandbox_efi.o
> > > >  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > >  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > +
> > > > +ifneq ($(MSYS_VERSION),0)
> > > > +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > +endif
> > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds 
> > > > b/arch/sandbox/cpu/u-boot-pe.lds
> > > > new file mode 100644
> > > > index ..031e70fafd03
> > > > --- /dev/null
> > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > @@ -0,0 +1,447 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +/*
> > > > + * U-Boot note: This was obtained by using the -verbose linker option. 
> > > > The
> > > > + * U-Boot additions are marked below.
> > > > + *
> > > > + * Ideally we would add sections to the executable, as is done with 
> > > > the Linux
> > > > + * build. But PE executables do not appear to work correctly if 
> > > > unexpected
> > > > + * sections are present:
> > > > + *
> > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: 
> > > > Exec format error
> > > > + *
> > > > + * So we take a approach of rewriting the whole file, for now. This 
> > > > will likely
> > > > + * break in the future when a toolchain change is made.
> > >
> > > Why not rather provide "layer" linker script which does this "rewriting"
> > > on top of the default linker script? With this way it is not needed to
> > > update linker script when a toolchain change it.
> > >
> >
> > How can we reliably do that, though? We don't really know the format
> > of the script and where to insert stuff.
> >
> > Regards,
> > Simon
>
> Well, I do not know what is the current issue. The proposed script in

See the comment at the top of the script:

+ * Ideally we would add sections to the executable, as is done with the Linux
+ * build. But PE 

Re: [PATCH v3 11/19] am64x: dts: binman: Package tiboot3.bin, tispl.bin u-boot.img

2023-04-25 Thread Simon Glass
Hi Neha,

On Tue, 25 Apr 2023 at 01:32, Neha Malcom Francis  wrote:
>
> Hi Simon
>
> On 25/04/23 01:12, Simon Glass wrote:
> > Hi Neha,
> >
> > On Fri, 21 Apr 2023 at 06:32, Neha Malcom Francis  wrote:
> >>
> >> Support added for HS and GP boot binaries for AM64x.
> >>
> >> tiboot3.bin, tispl.bin and u-boot.img: For HS-SE devices
> >> tiboot3.bin_fs, tispl.bin and u-boot.img: For HS-FS devices
> >> tiboot3.bin_unsigned, tispl.bin_unsigned, u-boot.img_unsigned: For GP
> >> devices
> >>
> >> Note that the bootflow followed by AM64x requires:
> >>
> >> tiboot3.bin:
> >>  * R5 SPL
> >>  * R5 SPL dtbs
> >>  * sysfw
> >>  * board-cfg
> >>  * pm-cfg
> >>  * sec-cfg
> >>  * rm-cfg
> >>
> >> tispl.bin:
> >>  * ATF
> >>  * OPTEE
> >>  * A53 SPL
> >>  * A53 SPL dtbs
> >>
> >> u-boot.img:
> >>  * A53 U-Boot
> >>  * A53 U-Boot dtbs
> >>
> >> Signed-off-by: Neha Malcom Francis 
> >> ---
> >>   arch/arm/dts/k3-am642-evm-u-boot.dtsi |   2 +
> >>   arch/arm/dts/k3-am642-r5-evm.dts  |   1 +
> >>   arch/arm/dts/k3-am64x-binman.dtsi | 569 ++
> >>   board/ti/am64x/Kconfig|   2 +
> >>   4 files changed, 574 insertions(+)
> >>   create mode 100644 arch/arm/dts/k3-am64x-binman.dtsi
> >
> > Reviewed-by: Simon Glass 
> >
> > I notice that some of the entries are optional. Do you actual make use
> > of this (i.e. that when they are missing binman removes the entries)?
> >
>
> So right now the build generates binaries for all three types: HS-FS,
> HS-SE and GP devices. It's not necessary for the user to provide
> component binaries for all three of them, say they only have GP SYSFW
> binaries available with them. So that was the reasoning behind putting
> those binaries as optional, we should not have a failed build in those
> cases. However binaries like DM and board-config binaries that are
> common between all three needs to be there so it's not optional.

The way 'optional' is supposed to work is that an etype decides that
an optional entry is absent and so marks it as absent. Then when
drop_absent() is called, the entry is removed from its section.

You can certainly add that functionality if you like.

But we must have a reliable signal for whether an image is valid or not.

Regards,
Simon


Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

2023-04-25 Thread Ramon Fried
On Tue, Apr 25, 2023 at 4:17 PM Eugen Hristev
 wrote:
>
> On 4/25/23 16:06, Eugen Hristev wrote:
> > Add minimal support for 8125B version.
> > Changes are based on the Linux driver.
> > Tested on Radxa Rock 5B Rk3588 board.
> >
> > Connection to a laptop worked fine in 100 Mbps mode.
> > 1000 Mbps mode is not working at the moment.
> >
> > Signed-off-by: Eugen Hristev 
> > ---
>
> The one thing that impacts all the rtl chips is the way the pci BAR is
> now mapped.
> I could not test this on another platform so help on this matter is
> appreciated.
>
> Thanks!
> Eugen
Let's wait a bit to see if someone can test it. why did you change the
mapping of the BAR ?


Re: [PATCH 2/2] net: phy: Make phy_interface_is_rgmii a switch statement

2023-04-25 Thread Ramon Fried
On Thu, Apr 13, 2023 at 9:07 PM Nishanth Menon  wrote:
>
> Recent commit 75d28899e3e9 ("net: phy: Synchronize PHY interface modes
> with Linux") reordered the enum definitions. This exposed a problem in
> range checking functions to identify the interface type. Though this
> specific api was'nt impacted (all the RGMII definitions remained within
> range), this experience should be used to never to have to face this
> kind of challenge again.
>
> While it is possible for the phy drivers to practically use the enum's
> directly, drivers such as dp83867, dp83869, marvell, micrel_ksz90x1 etc
> use the same.
>
> Reported-by: Tom Rini 
> Signed-off-by: Nishanth Menon 
> ---
>  include/phy.h | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/include/phy.h b/include/phy.h
> index 1c4dc23bc5ba..812694cf4a81 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -361,8 +361,16 @@ int get_phy_id(struct mii_dev *bus, int addr, int devad, 
> u32 *phy_id);
>   */
>  static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
>  {
> -   return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
> -   phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
> +   switch (phydev->interface) {
> +   case PHY_INTERFACE_MODE_RGMII:
> +   case PHY_INTERFACE_MODE_RGMII_ID:
> +   case PHY_INTERFACE_MODE_RGMII_RXID:
> +   case PHY_INTERFACE_MODE_RGMII_TXID:
> +   return 1;
> +   default:
> +   fallthrough;
> +   }
> +   return 0;
>  }
>
>  /**
> --
> 2.40.0
>
Reviewed-by: Ramon Fried 


Re: [PATCH v3 3/3] net: dhcp6: Add a sandbox test for dhcp6

2023-04-25 Thread Ramon Fried
On Tue, Apr 11, 2023 at 8:48 PM  wrote:
>
> From: Sean Edmond 
>
> Requires proper environment with DHCP6 server provisioned.
>
> Signed-off-by: Sean Edmond 
> ---
>  configs/sandbox_defconfig |  1 +
>  test/py/tests/test_net.py | 25 +
>  2 files changed, 26 insertions(+)
>
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index ca95b2c5d2..d7ceedd601 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -341,3 +341,4 @@ CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
>  CONFIG_CMD_2048=y
> +CONFIG_CMD_DHCP6=y
> diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
> index 9ca6743afd..0447c0b2e0 100644
> --- a/test/py/tests/test_net.py
> +++ b/test/py/tests/test_net.py
> @@ -29,6 +29,11 @@ env__net_uses_pci = True
>  # set to False.
>  env__net_dhcp_server = True
>
> +# True if a DHCPv6 server is attached to the network, and should be tested.
> +# If DHCPv6 testing is not possible or desired, this variable may be omitted 
> or
> +# set to False.
> +env__net_dhcp6_server = True
> +
>  # A list of environment variables that should be set in order to configure a
>  # static IP. If solely relying on DHCP, this variable may be omitted or set 
> to
>  # an empty list.
> @@ -58,6 +63,7 @@ env__net_nfs_readable_file = {
>  """
>
>  net_set_up = False
> +net6_set_up = False
>
>  def test_net_pre_commands(u_boot_console):
>  """Execute any commands required to enable network hardware.
> @@ -93,6 +99,25 @@ def test_net_dhcp(u_boot_console):
>  global net_set_up
>  net_set_up = True
>
> +@pytest.mark.buildconfigspec('cmd_dhcp6')
> +def test_net_dhcp6(u_boot_console):
> +"""Test the dhcp6 command.
> +
> +The boardenv_* file may be used to enable/disable this test; see the
> +comment at the beginning of this file.
> +"""
> +
> +test_dhcp6 = u_boot_console.config.env.get('env__net_dhcp6_server', 
> False)
> +if not test_dhcp6:
> +pytest.skip('No DHCP6 server available')
> +
> +u_boot_console.run_command('setenv autoload no')
> +output = u_boot_console.run_command('dhcp6')
> +assert 'DHCP6 client bound to ' in output
> +
> +global net6_set_up
> +net6_set_up = True
> +
>  @pytest.mark.buildconfigspec('net')
>  def test_net_setup_static(u_boot_console):
>  """Set up a static IP configuration.
> --
> 2.40.0
>
Reviewed-by: Ramon Fried 


Re: [PATCH v3 1/3] net: dhcp6: Add DHCPv6 (DHCP for IPv6)

2023-04-25 Thread Ramon Fried
On Tue, Apr 11, 2023 at 8:48 PM  wrote:
>
> From: Sean Edmond 
>
> Adds DHCPv6 protocol to u-boot.
>
> Allows for address assignement with DHCPv6 4-message exchange
> (SOLICIT->ADVERTISE->REQUEST->REPLY).  Includes DHCPv6 options
> required by RFC 8415.  Also adds DHCPv6 options required
> for PXE boot.
>
> Possible enhancements:
> - Duplicate address detection on DHCPv6 assigned address
> - IPv6 address assignement through SLAAC
> - Sending/parsing other DHCPv6 options (NTP, DNS, etc...)
>
> Signed-off-by: Sean Edmond 
> ---
>  include/net.h |   6 +-
>  net/Makefile  |   1 +
>  net/dhcpv6.c  | 719 ++
>  net/dhcpv6.h  | 256 ++
>  net/net.c |  11 +-
>  5 files changed, 989 insertions(+), 4 deletions(-)
>  create mode 100644 net/dhcpv6.c
>  create mode 100644 net/dhcpv6.h
>
> diff --git a/include/net.h b/include/net.h
> index 399af5e064..181a6e3b13 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -484,6 +484,8 @@ extern char net_hostname[32];   /* Our hostname */
>  #ifdef CONFIG_NET
>  extern charnet_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN];  /* Our root 
> path */
>  #endif
> +/* Indicates whether the pxe path prefix / config file was specified in dhcp 
> option */
> +extern char *pxelinux_configfile;
>  /** END OF BOOTP EXTENTIONS **/
>  extern u8  net_ethaddr[ARP_HLEN];  /* Our ethernet 
> address */
>  extern u8  net_server_ethaddr[ARP_HLEN];   /* Boot server enet 
> address */
> @@ -504,8 +506,8 @@ extern ushort   net_native_vlan;/* 
> Our Native VLAN */
>  extern int net_restart_wrap;   /* Tried all network devices 
> */
>
>  enum proto_t {
> -   BOOTP, RARP, ARP, TFTPGET, DHCP, PING, PING6, DNS, NFS, CDP, NETCONS,
> -   SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI, WGET
> +   BOOTP, RARP, ARP, TFTPGET, DHCP, DHCP6, PING, PING6, DNS, NFS, CDP,
> +   NETCONS, SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI, 
> WGET
>  };
>
>  extern charnet_boot_file_name[1024];/* Boot File name */
> diff --git a/net/Makefile b/net/Makefile
> index bea000b206..5968110170 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_IPV6) += net6.o
>  obj-$(CONFIG_CMD_NFS)  += nfs.o
>  obj-$(CONFIG_CMD_PING) += ping.o
>  obj-$(CONFIG_CMD_PING6) += ping6.o
> +obj-$(CONFIG_CMD_DHCP6) += dhcpv6.o
>  obj-$(CONFIG_CMD_PCAP) += pcap.o
>  obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
> diff --git a/net/dhcpv6.c b/net/dhcpv6.c
> new file mode 100644
> index 00..0d1c600632
> --- /dev/null
> +++ b/net/dhcpv6.c
> @@ -0,0 +1,719 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) Microsoft Corporation
> + * Author: Sean Edmond 
> + *
> + */
> +
> +/* Simple DHCP6 network layer implementation. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "net_rand.h"
> +#include "dhcpv6.h"
> +
> +#define PORT_DHCP6_S   547 /* DHCP6 server UDP port */
> +#define PORT_DHCP6_C   546 /* DHCP6 client UDP port */
> +
> +/* default timeout parameters (in ms) */
> +#define SOL_MAX_DELAY_MS   1000
> +#define SOL_TIMEOUT_MS 1000
> +#define SOL_MAX_RT_MS  360
> +#define REQ_TIMEOUT_MS 1000
> +#define REQ_MAX_RT_MS  3
> +#define REQ_MAX_RC 10
> +#define MAX_WAIT_TIME_MS   6
> +
> +/* global variable to track any updates from DHCP6 server */
> +int updated_sol_max_rt_ms = SOL_MAX_RT_MS;
> +/* state machine parameters/variables */
> +struct dhcp6_sm_params sm_params;
> +
> +static void dhcp6_state_machine(bool timeout, uchar *rx_pkt, unsigned int 
> len);
> +
> +/* Handle DHCP received packets (set as UDP handler) */
> +static void dhcp6_handler(uchar *pkt, unsigned int dest, struct in_addr sip,
> + unsigned int src, unsigned int len)
> +{
> +   /* return if ports don't match DHCPv6 ports */
> +   if (dest != PORT_DHCP6_C || src != PORT_DHCP6_S)
> +   return;
> +
> +   dhcp6_state_machine(false, pkt, len);
> +}
> +
> +/**
> + * dhcp6_add_option() - Adds DHCP6 option to a packet
> + * @option_id: The option ID to add (See DHCP6_OPTION_* definitions)
> + * @pkt: A pointer to the current write location of the TX packet
> + *
> + * Return: The number of bytes written into "*pkt"
> + */
> +static int dhcp6_add_option(int option_id, uchar *pkt)
> +{
> +   struct dhcp6_option_duid_ll *duid_opt;
> +   struct dhcp6_option_elapsed_time *elapsed_time_opt;
> +   struct dhcp6_option_ia_ta *ia_ta_opt;
> +   struct dhcp6_option_ia_na *ia_na_opt;
> +   struct dhcp6_option_oro *oro_opt;
> +   struct dhcp6_option_client_arch *client_arch_opt;
> +   struct dhcp6_option_vendor_class *vendor_class_opt;
> +   int opt_len;
> +   long elapsed_time;
> +   size_t vci_strlen;
> +   int num_oro = 0;
> +   int num_client_arch = 0;
> + 

Re: [PATCH v3 2/3] net: dhcp6: pxe: Add DHCP/PXE commands for IPv6

2023-04-25 Thread Ramon Fried
On Tue, Apr 11, 2023 at 8:48 PM  wrote:
>
> From: Sean Edmond 
>
> Adds commands to support DHCP and PXE with IPv6.
>
> New configs added:
> - CMD_DHCP6
> - DHCP6_PXE_CLIENTARCH
> - DHCP6_PXE_DHCP_OPTION
> - DHCP6_ENTERPRISE_ID
>
> New commands added (when IPv6 is enabled):
> - dhcp6
> - pxe get -ipv6
> - pxe boot -ipv6
>
> Signed-off-by: Sean Edmond 
> ---
>  boot/bootmeth_distro.c |  2 +-
>  boot/bootmeth_pxe.c|  4 +-
>  boot/pxe_utils.c   |  3 +-
>  cmd/Kconfig| 26 +
>  cmd/net.c  | 23 
>  cmd/pxe.c  | 85 ++
>  cmd/sysboot.c  |  2 +-
>  include/pxe_utils.h| 10 -
>  8 files changed, 132 insertions(+), 23 deletions(-)
>
> diff --git a/boot/bootmeth_distro.c b/boot/bootmeth_distro.c
> index 356929828b..b4b73ecbf5 100644
> --- a/boot/bootmeth_distro.c
> +++ b/boot/bootmeth_distro.c
> @@ -150,7 +150,7 @@ static int distro_boot(struct udevice *dev, struct 
> bootflow *bflow)
> info.dev = dev;
> info.bflow = bflow;
> ret = pxe_setup_ctx(, , distro_getfile, , true,
> -   bflow->subdir);
> +   bflow->subdir, false);
> if (ret)
> return log_msg_ret("ctx", -EINVAL);
>
> diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
> index ecf8557af8..5a8af2bbd0 100644
> --- a/boot/bootmeth_pxe.c
> +++ b/boot/bootmeth_pxe.c
> @@ -70,7 +70,7 @@ static int distro_pxe_read_bootflow(struct udevice *dev, 
> struct bootflow *bflow)
> addr = simple_strtoul(addr_str, NULL, 16);
>
> log_debug("calling pxe_get()\n");
> -   ret = pxe_get(addr, , );
> +   ret = pxe_get(addr, , , false);
> log_debug("pxe_get() returned %d\n", ret);
> if (ret)
> return log_msg_ret("pxeb", ret);
> @@ -146,7 +146,7 @@ static int distro_pxe_boot(struct udevice *dev, struct 
> bootflow *bflow)
> info.bflow = bflow;
> info.cmdtp = 
> ret = pxe_setup_ctx(ctx, , distro_pxe_getfile, , false,
> -   bflow->subdir);
> +   bflow->subdir, false);
> if (ret)
> return log_msg_ret("ctx", -EINVAL);
>
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index 3a1e50f2b1..d13c47dd94 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -1578,7 +1578,7 @@ void handle_pxe_menu(struct pxe_context *ctx, struct 
> pxe_menu *cfg)
>
>  int pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp,
>   pxe_getfile_func getfile, void *userdata,
> - bool allow_abs_path, const char *bootfile)
> + bool allow_abs_path, const char *bootfile, bool use_ipv6)
>  {
> const char *last_slash;
> size_t path_len = 0;
> @@ -1588,6 +1588,7 @@ int pxe_setup_ctx(struct pxe_context *ctx, struct 
> cmd_tbl *cmdtp,
> ctx->getfile = getfile;
> ctx->userdata = userdata;
> ctx->allow_abs_path = allow_abs_path;
> +   ctx->use_ipv6 = use_ipv6;
>
> /* figure out the boot directory, if there is one */
> if (bootfile && strlen(bootfile) >= MAX_TFTP_PATH_LEN)
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index e45b8847ae..460f29883a 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1673,6 +1673,15 @@ config CMD_DHCP
> help
>   Boot image via network using DHCP/TFTP protocol
>
> +config CMD_DHCP6
> +   bool "dhcp6"
> +   depends on IPV6
> +   help
> + Boot image via network using DHCPv6/TFTP protocol using IPv6.
> +
> + Will perform 4-message exchange with DHCPv6 server, requesting
> + the minimum required options to TFTP boot. Complies with RFC 8415.
> +
>  config BOOTP_MAY_FAIL
> bool "Allow for the BOOTP/DHCP server to not be found"
> depends on CMD_BOOTP
> @@ -1786,6 +1795,23 @@ config BOOTP_VCI_STRING
> default "U-Boot.arm" if ARM
> default "U-Boot"
>
> +if CMD_DHCP6
> +
> +config DHCP6_PXE_CLIENTARCH
> +   hex
> +   default 0x16 if ARM64
> +   default 0x15 if ARM
> +   default 0xFF
> +
> +config DHCP6_PXE_DHCP_OPTION
> +   bool "Request & store 'pxe_configfile' from DHCP6 server"
> +
> +config DHCP6_ENTERPRISE_ID
> +   int "Enterprise ID to send in DHCPv6 Vendor Class Option"
> +   default 0
> +
> +endif
> +
>  config CMD_TFTPBOOT
> bool "tftpboot"
> default y
> diff --git a/cmd/net.c b/cmd/net.c
> index d5e20843dd..76fd2e7d34 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -111,6 +111,29 @@ U_BOOT_CMD(
>  );
>  #endif
>
> +#if defined(CONFIG_CMD_DHCP6)
> +static int do_dhcp6(struct cmd_tbl *cmdtp, int flag, int argc,
> +   char *const argv[])
> +{
> +   int i;
> +   int dhcp_argc;
> +   char *dhcp_argv[] = {NULL, NULL, NULL, NULL};
> +
> +   /* Add -ipv6 flag for autoload */
> +   for (i = 0; i < argc; i++)
> +   dhcp_argv[i] = argv[i];
> +   

Re: [PATCH v2 3/3] net: dhcp6: Add a sandbox test for dhcp6

2023-04-25 Thread Ramon Fried
On Fri, Apr 7, 2023 at 9:55 PM Simon Glass  wrote:
>
> On Fri, 7 Apr 2023 at 18:56,  wrote:
> >
> > From: Sean Edmond 
> >
> > Requires proper environment with DHCP6 server provisioned.
> >
> > Signed-off-by: Sean Edmond 
> > ---
> >  configs/sandbox_defconfig |  1 +
> >  test/py/tests/test_net.py | 25 +
> >  2 files changed, 26 insertions(+)
>
> Reviewed-by: Simon Glass 
Acked-by: Ramon Fried 


Re: [PATCH v2 1/3] net: dhcp6: Add DHCPv6 (DHCP for IPv6)

2023-04-25 Thread Ramon Fried
On Fri, Apr 7, 2023 at 9:55 PM Simon Glass  wrote:
>
> Hi,
>
> On Fri, 7 Apr 2023 at 18:56,  wrote:
> >
> > From: Sean Edmond 
> >
> > Adds DHCPv6 protocol to u-boot.
> >
> > Allows for address assignement with DHCPv6 4-message exchange
> > (SOLICIT->ADVERTISE->REQUEST->REPLY).  Includes DHCPv6 options
> > required by RFC 8415.  Also adds DHCPv6 options required
> > for PXE boot.
> >
> > Possible enhancements:
> > - Duplicate address detection on DHCPv6 assigned address
> > - IPv6 address assignement through SLAAC
> > - Sending/parsing other DHCPv6 options (NTP, DNS, etc...)
> >
> > Signed-off-by: Sean Edmond 
> > ---
> >  include/net.h |   8 +-
> >  net/Makefile  |   1 +
> >  net/dhcpv6.c  | 735 ++
> >  net/dhcpv6.h  | 212 +++
> >  net/net.c |  12 +
> >  5 files changed, 966 insertions(+), 2 deletions(-)
> >  create mode 100644 net/dhcpv6.c
> >  create mode 100644 net/dhcpv6.h
>
> This looks good to me. I just have a few nits below. With those fixed:
>
> Reviewed-by: Simon Glass 
>
> >
> > diff --git a/include/net.h b/include/net.h
> > index 399af5e064..cac818e292 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -484,6 +484,10 @@ extern charnet_hostname[32];   /* Our 
> > hostname */
> >  #ifdef CONFIG_NET
> >  extern charnet_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN];  /* Our root 
> > path */
> >  #endif
> > +#if defined(CONFIG_DHCP6_PXE_DHCP_OPTION)
>
> You can drop this #ifdef as any reference to a non-existent var will
> give a build error.
>
> > +/* Indicates whether the pxe path prefix / config file was specified in 
> > dhcp option */
> > +extern char *pxelinux_configfile;
> > +#endif
> >  /** END OF BOOTP EXTENTIONS **/
> >  extern u8  net_ethaddr[ARP_HLEN];  /* Our ethernet 
> > address */
> >  extern u8  net_server_ethaddr[ARP_HLEN];   /* Boot server enet 
> > address */
> > @@ -504,8 +508,8 @@ extern ushort   net_native_vlan;/* 
> > Our Native VLAN */
> >  extern int net_restart_wrap;   /* Tried all network 
> > devices */
> >
> >  enum proto_t {
> > -   BOOTP, RARP, ARP, TFTPGET, DHCP, PING, PING6, DNS, NFS, CDP, 
> > NETCONS,
> > -   SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI, WGET
> > +   BOOTP, RARP, ARP, TFTPGET, DHCP, DHCP6, PING, PING6, DNS, NFS, CDP,
> > +   NETCONS, SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, 
> > NCSI, WGET
> >  };
> >
> >  extern charnet_boot_file_name[1024];/* Boot File name */
> > diff --git a/net/Makefile b/net/Makefile
> > index bea000b206..5968110170 100644
> > --- a/net/Makefile
> > +++ b/net/Makefile
> > @@ -22,6 +22,7 @@ obj-$(CONFIG_IPV6) += net6.o
> >  obj-$(CONFIG_CMD_NFS)  += nfs.o
> >  obj-$(CONFIG_CMD_PING) += ping.o
> >  obj-$(CONFIG_CMD_PING6) += ping6.o
> > +obj-$(CONFIG_CMD_DHCP6) += dhcpv6.o
> >  obj-$(CONFIG_CMD_PCAP) += pcap.o
> >  obj-$(CONFIG_CMD_RARP) += rarp.o
> >  obj-$(CONFIG_CMD_SNTP) += sntp.o
> > diff --git a/net/dhcpv6.c b/net/dhcpv6.c
> > new file mode 100644
> > index 00..9204909c1f
> > --- /dev/null
> > +++ b/net/dhcpv6.c
> > @@ -0,0 +1,735 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) Microsoft Corporation
> > + * Author: Sean Edmond 
> > + *
> > + */
> > +
> > +/* Simple DHCP6 network layer implementation. */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include "dhcpv6.h"
> > +#include 
> > +#include 
> > +#include "net_rand.h"
>
> Please fix header order:
> https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
>
> > +
> > +#define PORT_DHCP6_S   547 /* DHCP6 server UDP port */
> > +#define PORT_DHCP6_C   546 /* DHCP6 client UDP port */
> > +
> > +/* default timeout parameters (in ms) */
> > +#define SOL_MAX_DELAY_MS   1000
> > +#define SOL_TIMEOUT_MS 1000
> > +#define SOL_MAX_RT_MS  360
> > +#define REQ_TIMEOUT_MS 1000
> > +#define REQ_MAX_RT_MS  3
> > +#define REQ_MAX_RC 10
> > +#define MAX_WAIT_TIME_MS   6
> > +
> > +/* global variable to track any updates from DHCP6 server */
> > +int updated_sol_max_rt_ms = SOL_MAX_RT_MS;
> > +
> > +static void dhcp6_timeout_handler(void);
> > +static void dhcp6_state_machine(bool timeout, uchar *rx_pkt, unsigned int 
> > len);
> > +static void dhcp6_parse_options(uchar *rx_pkt, unsigned int len);
>
> Rather than forward decls can you reorder the functions?
>
> > +
> > +struct dhcp6_sm_params sm_params;
> > +
> > +/*
> > + * Handle DHCP received packets (set as UDP handler)
> > + */
>
> Please check single-line comment style
>
> > +static void dhcp6_handler(uchar *pkt, unsigned int dest, struct in_addr 
> > sip,
> > + unsigned int src, unsigned int len)
> > +{
> > +   /* return if ports don't 

Re: [PATCH v3 11/19] am64x: dts: binman: Package tiboot3.bin, tispl.bin u-boot.img

2023-04-25 Thread Andrew Davis

On 4/25/23 2:31 AM, Neha Malcom Francis wrote:

Hi Simon

On 25/04/23 01:12, Simon Glass wrote:

Hi Neha,

On Fri, 21 Apr 2023 at 06:32, Neha Malcom Francis  wrote:


Support added for HS and GP boot binaries for AM64x.

tiboot3.bin, tispl.bin and u-boot.img: For HS-SE devices
tiboot3.bin_fs, tispl.bin and u-boot.img: For HS-FS devices
tiboot3.bin_unsigned, tispl.bin_unsigned, u-boot.img_unsigned: For GP
devices

Note that the bootflow followed by AM64x requires:

tiboot3.bin:
 * R5 SPL
 * R5 SPL dtbs
 * sysfw
 * board-cfg
 * pm-cfg
 * sec-cfg
 * rm-cfg

tispl.bin:
 * ATF
 * OPTEE
 * A53 SPL
 * A53 SPL dtbs

u-boot.img:
 * A53 U-Boot
 * A53 U-Boot dtbs

Signed-off-by: Neha Malcom Francis 
---
  arch/arm/dts/k3-am642-evm-u-boot.dtsi |   2 +
  arch/arm/dts/k3-am642-r5-evm.dts  |   1 +
  arch/arm/dts/k3-am64x-binman.dtsi | 569 ++
  board/ti/am64x/Kconfig    |   2 +
  4 files changed, 574 insertions(+)
  create mode 100644 arch/arm/dts/k3-am64x-binman.dtsi


Reviewed-by: Simon Glass 

I notice that some of the entries are optional. Do you actual make use
of this (i.e. that when they are missing binman removes the entries)?



So right now the build generates binaries for all three types: HS-FS, HS-SE and 
GP devices. It's not necessary for the user to provide component binaries for 
all three of them, say they only have GP SYSFW binaries available with them.


Would this ever need to happen? We provide all three firmware types for all our
SoCs out in public[0], why would anyone only have one type of firmware 
available?

Andrew

[0] 
https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/tree/ti-sysfw?h=ti-linux-firmware


So that was the reasoning behind putting those binaries as optional, we should 
not have a failed build in those cases. However binaries like DM and 
board-config binaries that are common between all three needs to be there so 
it's not optional.


Regards,
Simon




Re: [PATCH 00/31] Allow building sandbox with MSYS2

2023-04-25 Thread Simon Glass
Hi Tom,

On Tue, 25 Apr 2023 at 10:04, Tom Rini  wrote:
>
> On Tue, Apr 25, 2023 at 04:59:53AM +0200, Heinrich Schuchardt wrote:
> >
> >
> > Am 25. April 2023 01:08:05 MESZ schrieb Simon Glass :
> > >This expands the existing work to allow sandbox to build and run on
> > >Windows using MSYS2.
> >
> > Why do we need this?
> > Wouldn't a developer on Windows be much better served using WSL 
> > (https://learn.microsoft.com/en-us/windows/wsl/install)?
>
> Depending on corporate rules that may or may not be allowed. But I too
> am interested in the use case driving these changes. Sandbox is
> essentially a test platform. We test that we can do host tools on
> Windows via MSYS2 for semi-reasonable reasons, since those tools can be
> used in a number of places. We do the same on macOS in Azure as a check
> on building regular targets there too (which should work and I believe
> people do, both from macOS and from *BSD which this is a stand-in for,
> as we can't get free VMs for them today). What's the end goal of this
> series?

It is really just an extension of the existing tools-only build. I am
not sure it is strongly motivated, but the end goal is to be able to
do small features / development on U-Boot on Windows, since some users
seem to only use a 'pure' Windows environment and don't use WSL, which
after all is for cross-compiling. For example, it is possible to build
a FIT and check it using the tools, but it is not possible (without
this series) to see whether U-Boot will load it and behave correctly.

The binman changes are to make that work on Windows (pip install
binary-manager) which is important for that tool.

The 'weak' issue is something that I am hoping can be resolved (in
fact Bin made a promising comment on that patch).

This has all come up as we try to establish a standard 'payload'
format which can be used across the industry. For better or worse,
some people do use Windows for development.

Regards,
Simon


[PATCH v2] common/Kconfig: fix comments syntax error

2023-04-25 Thread Hugo Villeneuve
From: Hugo Villeneuve 

Fix comments error in EVENT_DEBUG description:
this get usefui -> this to get useful

Signed-off-by: Hugo Villeneuve 
---
 common/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/Kconfig b/common/Kconfig
index f2783ee65d..0d69e83a33 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -626,7 +626,7 @@ config EVENT_DEBUG
bool "Enable event debugging assistance"
default y if SANDBOX
help
- Enable this get usefui features for seeing what is happening with
+ Enable this to get useful features for seeing what is happening with
  events, such as event-type names. This adds to the code size of
  U-Boot so can be turned off for production builds.
 
-- 
2.30.2



Re: [PATCH] common/Kconfig: fix comments syntax error

2023-04-25 Thread Hugo Villeneuve
On Tue, 25 Apr 2023 12:00:58 -0600
Simon Glass  wrote:

> Hi Hugo,
> 
> On Tue, 25 Apr 2023 at 07:46, Hugo Villeneuve  wrote:
> >
> > From: Hugo Villeneuve 
> >
> > Fix comments error in EVENT_DEBUG description:
> > usefui -> useful
> >
> > Signed-off-by: Hugo Villeneuve 
> > ---
> >  common/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index f2783ee65d..8c5b672cdf 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -626,7 +626,7 @@ config EVENT_DEBUG
> > bool "Enable event debugging assistance"
> > default y if SANDBOX
> > help
> > - Enable this get usefui features for seeing what is happening with
> > + Enable this get useful features for seeing what is happening with
> 
> I think 'Enable this to get useful' would be better

Yes agreed, will resubmit.

Thank you.
Hugo.

> 
> >   events, such as event-type names. This adds to the code size of
> >   U-Boot so can be turned off for production builds.
> >
> > --
> > 2.30.2
> >
> 
> Reviewed-by: Simon Glass 
> 
> Regards,
> Simon


Re: [PATCH 0/6] virtio: Use bounce buffers when VIRTIO_F_IOMMU_PLATFORM set

2023-04-25 Thread Tom Rini
On Wed, 29 Mar 2023 22:24:54 +0800, Ying-Chun Liu (PaulLiu) wrote:

> These patches will use bounce buffers when VIRTIO_F_IOMMU_PLATFORM feature
> is in a virtio device.
> 
> This feature can be tested with qemu with -device virtio-iommu-pci.
> So that when a -device virtio-blk-pci with iommu_platform=true, it will
> uses the bounce buffer instead.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom



Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

2023-04-25 Thread Pali Rohár
On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 10:21, Pali Rohár  wrote:
> >
> > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > Add a script to allow the U-Boot sandbox executable to be built for
> > > Windows. Add a note as to why this seems to be necessary for now.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > >
> > >  Makefile   |  11 +-
> > >  arch/sandbox/config.mk |   4 +
> > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +
> > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > >
> > > diff --git a/Makefile b/Makefile
> > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1730,6 +1730,13 @@ else
> > >  u-boot-keep-syms-lto :=
> > >  endif
> > >
> > > +ifeq ($(MSYS_VERSION),0)
> > > +add_ld_script := -T u-boot.lds
> > > +else
> > > +add_ld_script := u-boot.lds
> > > +$(warning msys)
> > > +endif
> > > +
> > >  # Rule to link u-boot
> > >  # May be overridden by arch/$(ARCH)/config.mk
> > >  ifeq ($(LTO_ENABLE),y)
> > > @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > >   $(CC) -nostdlib -nostartfiles   
> > > \
> > >   $(LTO_FINAL_LDFLAGS) $(c_flags) 
> > > \
> > >   $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@   
> > > \
> > > - -T u-boot.lds $(u-boot-init)
> > > \
> > > + $(add_ld_script) $(u-boot-init) 
> > > \
> > >   -Wl,--whole-archive 
> > > \
> > >   $(u-boot-main)  
> > > \
> > >   $(u-boot-keep-syms-lto) 
> > > \
> > > @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> > >  else
> > >  quiet_cmd_u-boot__ ?= LD  $@
> > >cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@
> > > \
> > > - -T u-boot.lds $(u-boot-init)
> > > \
> > > + $(add_ld_script) $(u-boot-init) 
> > > \
> > >   --whole-archive 
> > > \
> > >   $(u-boot-main)  
> > > \
> > >   --no-whole-archive  
> > > \
> > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > index 2d184c5f652a..e05daf57ef8e 100644
> > > --- a/arch/sandbox/config.mk
> > > +++ b/arch/sandbox/config.mk
> > > @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
> > >  EFI_RELOC := reloc_sandbox_efi.o
> > >  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > >  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > +
> > > +ifneq ($(MSYS_VERSION),0)
> > > +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > +endif
> > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds 
> > > b/arch/sandbox/cpu/u-boot-pe.lds
> > > new file mode 100644
> > > index ..031e70fafd03
> > > --- /dev/null
> > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > @@ -0,0 +1,447 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * U-Boot note: This was obtained by using the -verbose linker option. 
> > > The
> > > + * U-Boot additions are marked below.
> > > + *
> > > + * Ideally we would add sections to the executable, as is done with the 
> > > Linux
> > > + * build. But PE executables do not appear to work correctly if 
> > > unexpected
> > > + * sections are present:
> > > + *
> > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec 
> > > format error
> > > + *
> > > + * So we take a approach of rewriting the whole file, for now. This will 
> > > likely
> > > + * break in the future when a toolchain change is made.
> >
> > Why not rather provide "layer" linker script which does this "rewriting"
> > on top of the default linker script? With this way it is not needed to
> > update linker script when a toolchain change it.
> >
> 
> How can we reliably do that, though? We don't really know the format
> of the script and where to insert stuff.
> 
> Regards,
> Simon

Well, I do not know what is the current issue. The proposed script in
your patch looks like it is copied from some binutils version and then
modified. Also I do not know from which binutils you have copied and
what modification you have done in it. So I cannot react or comment
anymore more here.

My idea is that to write those modifications into new layer script.


Re: [PATCH] common/Kconfig: fix comments syntax error

2023-04-25 Thread Simon Glass
Hi Hugo,

On Tue, 25 Apr 2023 at 07:46, Hugo Villeneuve  wrote:
>
> From: Hugo Villeneuve 
>
> Fix comments error in EVENT_DEBUG description:
> usefui -> useful
>
> Signed-off-by: Hugo Villeneuve 
> ---
>  common/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index f2783ee65d..8c5b672cdf 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -626,7 +626,7 @@ config EVENT_DEBUG
> bool "Enable event debugging assistance"
> default y if SANDBOX
> help
> - Enable this get usefui features for seeing what is happening with
> + Enable this get useful features for seeing what is happening with

I think 'Enable this to get useful' would be better

>   events, such as event-type names. This adds to the code size of
>   U-Boot so can be turned off for production builds.
>
> --
> 2.30.2
>

Reviewed-by: Simon Glass 

Regards,
Simon


Re: [PATCH v3 01/19] binman: ti-board-config: Add support for TI board config binaries

2023-04-25 Thread Simon Glass
Hi Neha,

On Tue, 25 Apr 2023 at 00:24, Neha Malcom Francis  wrote:
>
> Hi Simon
>
> On 25/04/23 01:12, Simon Glass wrote:
> > Hi Neha,
> >
> > On Fri, 21 Apr 2023 at 06:32, Neha Malcom Francis  wrote:
> >>
> >> The ti-board-config entry loads and validates a given YAML config file
> >> against a given schema, and generates the board config binary. K3
> >> devices require these binaries to be packed into the final system
> >> firmware images.
> >>
> >> Signed-off-by: Neha Malcom Francis 
> >> ---
> >>   tools/binman/entries.rst  |  48 
> >>   tools/binman/etype/ti_board_config.py | 269 ++
> >>   tools/binman/ftest.py |  32 +++
> >>   tools/binman/pyproject.toml   |   2 +-
> >>   tools/binman/test/277_ti_board_cfg.dts|  11 +
> >>   .../binman/test/278_ti_board_cfg_combined.dts |  25 ++
> >>   .../binman/test/279_ti_board_cfg_no_type.dts  |  11 +
> >>   .../binman/test/280_ti_board_cfg_no_file.dts  |  11 +
> >>   .../281_ti_board_cfg_combined_no_file.dts |  13 +
> >>   tools/binman/test/yaml/config.yaml|  19 ++
> >>   tools/binman/test/yaml/schema.yaml|  51 
> >>   tools/binman/test/yaml/schema_notype.yaml |  40 +++
> >>   12 files changed, 531 insertions(+), 1 deletion(-)
> >>   create mode 100644 tools/binman/etype/ti_board_config.py
> >>   create mode 100644 tools/binman/test/277_ti_board_cfg.dts
> >>   create mode 100644 tools/binman/test/278_ti_board_cfg_combined.dts
> >>   create mode 100644 tools/binman/test/279_ti_board_cfg_no_type.dts
> >>   create mode 100644 tools/binman/test/280_ti_board_cfg_no_file.dts
> >>   create mode 100644 
> >> tools/binman/test/281_ti_board_cfg_combined_no_file.dts
> >>   create mode 100644 tools/binman/test/yaml/config.yaml
> >>   create mode 100644 tools/binman/test/yaml/schema.yaml
> >>   create mode 100644 tools/binman/test/yaml/schema_notype.yaml
> >>
> >
> > Reviewed-by: Simon Glass 
> >
> > My only real comment is that errors should produce an error rather
> > than just a warning. E.g. a schema-validation error should be fatal,
> > since it won't work.
> >
> > You can call self.Raise() when something goes wrong. The tests should
> > check for that instead of a warning.
> >
>
> Makes sense. But I'm not sure I understand when we create an fake binary
> and when we choose to throw errors? Either case we end up with
> non-working binary or no binary at all. I see both styles in existing
> etypes and I can't form a reasoning for when to do what.

OK I see.

If -M is not provided, we should produce an error
If -M is provided, entries with missing external blobs should set
'self.missing' to True, so binman knows and can report it.

When you output a warning, it really doesn't do anything. So far as
the user is concerned the thing succeeded. We need the exit code to
indicate the right thing here.

In other words, we have support in Binman for handling missing blobs
and we should use that.

Regards,
Simon


Re: [PATCH v3 1/2] serial: zynqmp: Fetch baudrate from dtb and update

2023-04-25 Thread Simon Glass
Hi Venkatesh,

On Tue, 25 Apr 2023 at 06:05, Venkatesh Yadav Abbarapu
 wrote:
>
> From: Algapally Santosh Sagar 
>
> The baudrate configured in .config is taken by default by serial. If
> change of baudrate is required then the .config needs to changed and
> u-boot recompilation is required or the u-boot environment needs to be
> updated.
>
> To avoid this, support is added to fetch the baudrate directly from the
> device tree file and update.
> The serial, prints the log with the configured baudrate in the dtb.
> The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
> $fdtfile env variable") is taken as reference for changing the default
> environment variable.
>
> The default environment stores the default baudrate value, When default
> baudrate and dtb baudrate are not same glitches are seen on the serial.
> So, the environment also needs to be updated with the dtb baudrate to
> avoid the glitches on the serial.
>
> Signed-off-by: Algapally Santosh Sagar 
> Signed-off-by: Venkatesh Yadav Abbarapu 
> ---
>  drivers/serial/serial-uclass.c | 32 +++
>  include/env_default.h  |  7 --
>  include/env_internal.h |  2 +-
>  include/fdtdec.h   |  8 +++
>  include/serial.h   |  1 +
>  lib/fdtdec.c   | 40 ++
>  6 files changed, 87 insertions(+), 3 deletions(-)

Can you please add something to doc/ for this feature?

>
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 067fae2614..d77d3bda36 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -154,12 +154,44 @@ static void serial_find_console_or_panic(void)
>  }
>  #endif /* CONFIG_SERIAL_PRESENT */
>
> +#ifdef CONFIG_SERIAL_DT_BAUD

Please drop this #ifdef

> +int serial_get_valid_baudrate(int baud)
> +{
> +   int i;
> +
> +   for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {
> +   if (baud == baudrate_table[i])
> +   return 0;
> +   }
> +
> +   return -EINVAL;
> +}
> +#endif
> +
>  /* Called prior to relocation */
>  int serial_init(void)
>  {
>  #if CONFIG_IS_ENABLED(SERIAL_PRESENT)
> serial_find_console_or_panic();
> gd->flags |= GD_FLG_SERIAL_READY;
> +#ifdef CONFIG_SERIAL_DT_BAUD

if (IS_ENABLED(CONFIG_SERIAL_DT_BAUD)

You also should add a Kconfig option. I suggest naming it
OF_SERIAL_BOARD since we typically put an OF_ prefix on such options.

> +   int ret = 0;
> +   char *ptr = _environment[0];
> +
> +   /*
> +* Fetch the baudrate from the dtb and update the value in the
> +* default environment.
> +*/
> +   ret = fdtdec_get_baud_from_dtb(gd->fdt_blob);
> +   if (ret != -EINVAL && ret != -EFAULT) {
> +   gd->baudrate = ret;
> +
> +   while (*ptr != '\0' && *(ptr + 1) != '\0')
> +   ptr++;
> +   ptr += 2;
> +   sprintf(ptr, "baudrate=%d", gd->baudrate);
> +   }
> +#endif
> serial_setbrg();
>  #endif
>
> diff --git a/include/env_default.h b/include/env_default.h
> index c0df39d62f..4f286ffc9e 100644
> --- a/include/env_default.h
> +++ b/include/env_default.h
> @@ -23,7 +23,7 @@ env_t embedded_environment 
> __UBOOT_ENV_SECTION__(environment) = {
> {
>  #elif defined(DEFAULT_ENV_INSTANCE_STATIC)
>  static char default_environment[] = {
> -#elif defined(DEFAULT_ENV_IS_RW)
> +#elif defined(CONFIG_DEFAULT_ENV_IS_RW)
>  char default_environment[] = {
>  #else
>  const char default_environment[] = {
> @@ -44,7 +44,7 @@ const char default_environment[] = {
>  #if defined(CONFIG_BOOTDELAY)
> "bootdelay="__stringify(CONFIG_BOOTDELAY)   "\0"
>  #endif
> -#if defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
> +#if !defined(CONFIG_SERIAL_DT_BAUD) && defined(CONFIG_BAUDRATE) && 
> (CONFIG_BAUDRATE >= 0)
> "baudrate=" __stringify(CONFIG_BAUDRATE)"\0"
>  #endif
>  #ifdef CONFIG_LOADS_ECHO
> @@ -120,6 +120,9 @@ const char default_environment[] = {
>  #endif
>  #ifdef CFG_EXTRA_ENV_SETTINGS
> CFG_EXTRA_ENV_SETTINGS
> +#endif
> +#ifdef CONFIG_SERIAL_DT_BAUD
> +   "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"

What is this for?

>  #endif
> "\0"
>  #else /* CONFIG_USE_DEFAULT_ENV_FILE */
> diff --git a/include/env_internal.h b/include/env_internal.h
> index 6a69494646..fcb464263f 100644
> --- a/include/env_internal.h
> +++ b/include/env_internal.h
> @@ -89,7 +89,7 @@ typedef struct environment_s {
>  extern env_t embedded_environment;
>  #endif /* ENV_IS_EMBEDDED */
>
> -#ifdef DEFAULT_ENV_IS_RW
> +#ifdef CONFIG_DEFAULT_ENV_IS_RW

What is this change for?

>  extern char default_environment[];
>  #else
>  extern const char default_environment[];
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index aa61a0fca1..48937a7a46 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -657,6 +657,14 @@ int 

Re: [PATCH v3 09/19] am65: dts: binman: Package tiboot3.bin, sysfw.itb, tispl.bin, u-boot.img

2023-04-25 Thread Simon Glass
Hi Neha,

On Tue, 25 Apr 2023 at 01:26, Neha Malcom Francis  wrote:
>
> Hi Simon,
>
> On 25/04/23 01:12, Simon Glass wrote:
> > Hi Neha,
> >
> > On Fri, 21 Apr 2023 at 06:32, Neha Malcom Francis  wrote:
> >>
> >> Support added for HS and GP boot binaries for AM65x.
> >>
> >> tiboot3.bin, sysfw.itb, tispl.bin and u-boot.img: For HS devices
> >> tiboot3.bin_unsigned, sysfw.itb, tispl.bin_unsigned,
> >> u-boot.img_unsigned: For GP devices
> >>
> >> Note that the bootflow followed by AM65x requires:
> >>
> >> tiboot3.bin:
> >>  * R5 SPL
> >>  * R5 SPL dtbs
> >> sysfw.itb:
> >>  * sysfw
> >>  * board-cfg
> >>  * pm-cfg
> >>  * sec-cfg
> >>  * rm-cfg
> >>
> >> tispl.bin:
> >>  * ATF
> >>  * OPTEE
> >>  * A53 SPL
> >>  * A53 SPL dtbs
> >>
> >> u-boot.img:
> >>  * A53 U-Boot
> >>  * A53 U-Boot dtbs
> >>
> >> Signed-off-by: Neha Malcom Francis 
> >> ---
> >>   arch/arm/dts/k3-am654-base-board-u-boot.dtsi  |   1 +
> >>   .../dts/k3-am654-r5-base-board-u-boot.dtsi|   1 +
> >>   arch/arm/dts/k3-am65x-binman.dtsi | 551 ++
> >>   board/ti/am65x/Kconfig|   2 +
> >>   4 files changed, 555 insertions(+)
> >>   create mode 100644 arch/arm/dts/k3-am65x-binman.dtsi
> >
> > Reviewed-by: Simon Glass 
> >
> > Is there any way that these boards could share a .dtsi with just the
> > different bits in each board's dtsi? There seems to be a lot of
> > duplicate.
> >
>
> Maybe a common k3-binman.dtsi, but the common nodes (where the filenames
> of both the generated file as well as component files are all common)
> would be the ti-board-config ones... and if we change that as per last
> review comment to point to board/ti//config.bin then we'd have
> that particular to each device as well right?
>
> Or do you mean to say we use #defines for all?

No I just mean having a common .dtsi as you say above.

You can still override things in that if you want to, by adding a
phandle and then referencing it in the board-specific file.

Regards,
Simon


Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

2023-04-25 Thread Simon Glass
Hi Pali,

On Tue, 25 Apr 2023 at 10:21, Pali Rohár  wrote:
>
> On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > Add a script to allow the U-Boot sandbox executable to be built for
> > Windows. Add a note as to why this seems to be necessary for now.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  Makefile   |  11 +-
> >  arch/sandbox/config.mk |   4 +
> >  arch/sandbox/cpu/u-boot-pe.lds | 447 +
> >  3 files changed, 460 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> >
> > diff --git a/Makefile b/Makefile
> > index dd3fcd1782e5..0aa97a2c3b48 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1730,6 +1730,13 @@ else
> >  u-boot-keep-syms-lto :=
> >  endif
> >
> > +ifeq ($(MSYS_VERSION),0)
> > +add_ld_script := -T u-boot.lds
> > +else
> > +add_ld_script := u-boot.lds
> > +$(warning msys)
> > +endif
> > +
> >  # Rule to link u-boot
> >  # May be overridden by arch/$(ARCH)/config.mk
> >  ifeq ($(LTO_ENABLE),y)
> > @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> >   $(CC) -nostdlib -nostartfiles 
> >   \
> >   $(LTO_FINAL_LDFLAGS) $(c_flags)   
> >   \
> >   $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@ 
> >   \
> > - -T u-boot.lds $(u-boot-init)  
> >   \
> > + $(add_ld_script) $(u-boot-init)   
> >   \
> >   -Wl,--whole-archive   
> >   \
> >   $(u-boot-main)
> >   \
> >   $(u-boot-keep-syms-lto)   
> >   \
> > @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO $@
> >  else
> >  quiet_cmd_u-boot__ ?= LD  $@
> >cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@  
> >   \
> > - -T u-boot.lds $(u-boot-init)  
> >   \
> > + $(add_ld_script) $(u-boot-init)   
> >   \
> >   --whole-archive   
> >   \
> >   $(u-boot-main)
> >   \
> >   --no-whole-archive
> >   \
> > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > index 2d184c5f652a..e05daf57ef8e 100644
> > --- a/arch/sandbox/config.mk
> > +++ b/arch/sandbox/config.mk
> > @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
> >  EFI_RELOC := reloc_sandbox_efi.o
> >  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> >  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > +
> > +ifneq ($(MSYS_VERSION),0)
> > +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > +endif
> > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > new file mode 100644
> > index ..031e70fafd03
> > --- /dev/null
> > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > @@ -0,0 +1,447 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > + * U-Boot additions are marked below.
> > + *
> > + * Ideally we would add sections to the executable, as is done with the 
> > Linux
> > + * build. But PE executables do not appear to work correctly if unexpected
> > + * sections are present:
> > + *
> > + *   $ /tmp/b/sandbox/u-boot.exe
> > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec 
> > format error
> > + *
> > + * So we take a approach of rewriting the whole file, for now. This will 
> > likely
> > + * break in the future when a toolchain change is made.
>
> Why not rather provide "layer" linker script which does this "rewriting"
> on top of the default linker script? With this way it is not needed to
> update linker script when a toolchain change it.
>

How can we reliably do that, though? We don't really know the format
of the script and where to insert stuff.

Regards,
Simon


Re: [PATCH v2] cosmetic:fix typo in 'mmc write' example

2023-04-25 Thread Simon Glass
Hi Alexander,

On Tue, 25 Apr 2023 at 04:00, Alexander Shirokov
 wrote:
>
> In the 'mmc write' command example, it writes 16 blocks (0x10). But the
> output contains 256 (0x100) blocks. This patch fixes the mismatch.
>
> Signed-off-by: Alexander Shirokov 
> ---
>  doc/usage/cmd/mmc.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/usage/cmd/mmc.rst b/doc/usage/cmd/mmc.rst
> index 55e3f9cf98..2ca1a5cf0c 100644
> --- a/doc/usage/cmd/mmc.rst
> +++ b/doc/usage/cmd/mmc.rst
> @@ -216,7 +216,7 @@ The raw data can be read/written via 'mmc read/write' 
> command:
>  => mmc read 0x4000 0x5000 0x100
>  MMC read: dev # 0, block # 20480, count 256 ... 256 blocks read: OK
>
> -=> mmc write 0x4000 0x5000 0x10
> +=> mmc write 0x4000 0x5000 100

Drop the other 0x things too?

>  MMC write: dev # 0, block # 20480, count 256 ... 256 blocks written: OK
>
>  The partition list can be shown via 'mmc part' command:
> --
> 2.40.0
>

Regards,
Simon


Re: [PATCH v3 6/6] video: panel: add generic endeavoru panel

2023-04-25 Thread Simon Glass
On Tue, 25 Apr 2023 at 01:52, Svyatoslav Ryhel  wrote:
>
> Family of panels used by HTC in One X. Though were used variants
> at least from 3 vendors, this driver provides generic support for
> all of them.
>
> Tested-by: Ion Agorria  # HTC One X T30 Sony
> Tested-by: Svyatoslav Ryhel  # HTC One X T30 Sharp
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  drivers/video/Kconfig   |  11 ++
>  drivers/video/Makefile  |   1 +
>  drivers/video/endeavoru-panel.c | 252 
>  3 files changed, 264 insertions(+)
>  create mode 100644 drivers/video/endeavoru-panel.c
>

Reviewed-by: Simon Glass 


Re: [PATCH] event: fix comments syntax error

2023-04-25 Thread Simon Glass
On Tue, 25 Apr 2023 at 07:47, Hugo Villeneuve  wrote:
>
> From: Hugo Villeneuve 
>
> Fix comments syntax error in event description:
> creasted -> created
>
> Signed-off-by: Hugo Villeneuve 
> ---
>  doc/develop/event.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/develop/event.rst b/doc/develop/event.rst
> index 4ff5934837..e60cbf6569 100644
> --- a/doc/develop/event.rst
> +++ b/doc/develop/event.rst
> @@ -12,7 +12,7 @@ Rather than using weak functions and direct calls across 
> subsystemss, it is
>  often easier to use an event.
>
>  An event consists of a type (e.g. EVT_DM_POST_INIT) and some optional data,
> -in `union event_data`. An event spy can be creasted to watch for events of a
> +in `union event_data`. An event spy can be created to watch for events of a
>  particular type. When the event is created, it is sent to each spy in turn.
>
>
> --
> 2.30.2
>

Reviewed-by: Simon Glass 


Re: [PATCH] mtd: spi-nor-core: Add fixups for s25fs512s

2023-04-25 Thread Jagan Teki
On Sun, Apr 23, 2023 at 5:26 AM Marek Vasut
 wrote:
>
> From: Takahiro Kuwano 
>
> This patch adds fixups for s25fs512s to address the following issues
> from reading SFDP:
>
>   - Non-uniform sectors by factory default. The setting needs to be
> checked and assign erase hook as needed.
>   - Page size is wrongly advertised in SFDP.
>   - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h)
> are not supported.
>   - Bank Address Register (BAR) is not supported.
>
> In addition, volatile version of Quad Enable is used for safety.
>
> Based on patch by Takahiro Kuwano with s25fs_s_post_bfpt_fixup() updated
> to use 4-byte address commands instead of extended address mode and the
> page_size is fixed to 256
>
> For future use, manufacturer code should be moved out from framework
> code as same as in Linux.
>
> Reviewed-by: Marek Vasut 
> Signed-off-by: Takahiro Kuwano 
> Signed-off-by: Hai Pham 
> Signed-off-by: Cong Dang 
> Signed-off-by: Marek Vasut 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH v2] cmd: sf/nand: Print and return failure when 0 length is passed

2023-04-25 Thread Jagan Teki
On Wed, Apr 12, 2023 at 1:22 PM Ashok Reddy Soma
 wrote:
>
> For sf commands, when '0' length is passed for erase, update, write or
> read, there might be undesired results. Ideally '0' length means nothing to
> do.
>
> So print 'ERROR: Invalid size 0' and return cmd failure when length '0' is
> passed to sf commands. Same thing applies for nand commands also.
>
> Example:
>
> ZynqMP> sf erase 0 0
> ERROR: Invalid size 0
> ZynqMP> sf write 1 0 0
> ERROR: Invalid size 0
> ZynqMP> sf read 1 0 0
> ERROR: Invalid size 0
> ZynqMP> sf update 1000 1 0
> ERROR: Invalid size 0
> ZynqMP>
>
> Signed-off-by: Ashok Reddy Soma 
> ---
>
> Changes in v2:
>  - Changed print from 'size is 0' to Invalid size 0 without quites.
>  - Modified description to be imperative
>  - Fixed typo in description from "samething" to "same thing"
>
>  cmd/legacy-mtd-utils.c | 5 +
>  cmd/sf.c   | 5 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/cmd/legacy-mtd-utils.c b/cmd/legacy-mtd-utils.c
> index ac7139f84d..61987918a4 100644
> --- a/cmd/legacy-mtd-utils.c
> +++ b/cmd/legacy-mtd-utils.c
> @@ -88,6 +88,11 @@ int mtd_arg_off_size(int argc, char *const argv[], int 
> *idx, loff_t *off,
> return -1;
> }
>
> +   if (*size == 0) {
> +   printf("ERROR: Invalid size 0\n");
> +   return -1;
> +   }
> +
>  print:
> printf("device %d ", *idx);
> if (*size == chipsize)
> diff --git a/cmd/sf.c b/cmd/sf.c
> index 11b9c25896..a6aadc2b00 100644
> --- a/cmd/sf.c
> +++ b/cmd/sf.c
> @@ -353,6 +353,11 @@ static int do_spi_flash_erase(int argc, char *const 
> argv[])
> if (ret != 1)
> return CMD_RET_USAGE;
>
> +   if (size == 0) {
> +   printf("ERROR: Invalid size 0\n");
> +   return CMD_RET_FAILURE;
> +   }

I feel it is too verbose and if erase happened it shows the log
otherwise it shows nothing, and that is enough.

Jagan.


Re: [PATCH] spi: synquacer: Silence unused variable warnings

2023-04-25 Thread Jagan Teki
On Fri, Apr 7, 2023 at 2:43 PM Ilias Apalodimas
 wrote:
>
> When building with clang, the compiler compains with
>
> drivers/spi/spi-synquacer.c:212:11: warning: variable 'bus_width' is used 
> uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> else if (priv->mode & SPI_TX_OCTAL)
>  ^
> drivers/spi/spi-synquacer.c:276:11: note: uninitialized use occurs here
> val |= ((bus_width >> 1) << BUS_WIDTH);
>  ^
> drivers/spi/spi-synquacer.c:212:7: note: remove the 'if' if its condition is 
> always true
> else if (priv->mode & SPI_TX_OCTAL)
>  ^~
> drivers/spi/spi-synquacer.c:189:25: note: initialize the variable 'bus_width' 
> to silence this warning
>
> So initialize bus_width to 1 and add a warning if none of the configured
> modes matches
>
> Signed-off-by: Ilias Apalodimas 
> ---

Applied to u-boot-spi/master


Re: [PATCH 1/1] mtd: spi-nor: missing fallthrough in set_4byte()

2023-04-25 Thread Jagan Teki
On Sat, Apr 1, 2023 at 1:04 PM Heinrich Schuchardt
 wrote:
>
> Add a missing fallthrough macro to avoid a -Wimplicit-fallthrough warning.
>
> Signed-off-by: Heinrich Schuchardt 
> ---

Applied to u-boot-spi/master


Re: [PATCH v1] spi: npcm-fiu: add regulator feature and remove set clock

2023-04-25 Thread Jagan Teki
On Tue, Mar 7, 2023 at 1:40 PM Jim Liu  wrote:
>
> NPCM7xx/NPCM8xx default is boot from flash.
> removed set clock feature due to reliability and security.
> the clock will set by bootblock or tip.
>
> Signed-off-by: Jim Liu 
> ---

Reviewed-by: Jagan Teki 

Applied to u-boot-spi/master


Re: [PATCH] spi: f-ospi: Add missing spi_mem_default_supports_op() helper

2023-04-25 Thread Jagan Teki
On Mon, Mar 27, 2023 at 11:05 AM Kunihiko Hayashi
 wrote:
>
> The .supports_op() callback function returns true by default after
> performing driver-specific checks. Therefore the driver cannot apply
> the buswidth in devicetree.
>
> Call spi_mem_default_supports_op() helper to handle the buswidth
> in devicetree.
>
> Fixes: 358f803ae21c ("spi: Add Socionext F_OSPI SPI flash controller driver")
> Signed-off-by: Kunihiko Hayashi 
> ---

Applied to u-boot-spi/master


Re: [PATCH] mtd: spi-nor: Add CHIP_ERASE optimization

2023-04-25 Thread Jagan Teki
On Thu, Mar 2, 2023 at 7:16 AM Marek Vasut  wrote:
>
> Add support for CHIP_ERASE opcode 0xc7 . This is useful in case the
> entire SPI NOR is supposed to be erase at once, as is it considerably
> faster than 4k sector erase and even slightly faster than 64k block
> erase. The spi_nor_erase_chip() implementation is adapted from Linux
> 6.1.y as of commit 7d54cb2c26dad ("Linux 6.1.14") . The chip erase is
> only used in case the entire MTD device is being erased, and the chip
> does support this functionality.
>
> Timing figures from W25Q128JW:
> 16 MiB erase using 4kiB sector erase opcode 0x20 ... 107.5s
> 16 MiB erase using 64kiB block erase opcode 0xd8 ... 39.1s
> 16 MiB erase using chip erase opcode 0xc7 .. 38.7s
>
> Signed-off-by: Marek Vasut 
> ---

Reviewed-by: Jagan Teki 

Applied to u-boot-spi/master


Re: [PATCH] environment: ti: Add get_fit_config command to get FIT config string

2023-04-25 Thread Tom Rini
On Tue, Apr 25, 2023 at 11:20:45AM -0500, Andrew Davis wrote:

> When OE is packaging a dtb file into the FIT image it names the node based
> on the dtb filename. Node names can't have "/" so it is turned into "_".
> We select our FIT config using the "fdtfile" env var so we don't duplicate
> the board_name to fdt logic. Result is fdtfile needs mangled when used to
> select a config node from OE made FIT image. Do this here.
> 
> Signed-off-by: Andrew Davis 

Thanks for getting this done!

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v2 2/2] ARM: dts: stm32mp: alignment with v6.3

2023-04-25 Thread Marek Vasut

On 4/24/23 16:21, Patrick Delaunay wrote:

Device tree alignment with Linux kernel v6.3:
- f5a058023239 - ARM: dts: stm32: add i2c nodes into stm32mp131.dtsi
- 8539ebb435a5 - ARM: dts: stm32: enable i2c1 and i2c5 on
   stm32mp135f-dk.dts
- 8539ebb435a5 - ARM: dts: stm32: add spi nodes into stm32mp131.dtsi
- 15f72e0da4da - ARM: dts: stm32: add pinctrl and disabled spi5 node in
   stm32mp135f-dk
- ea99a5a02ebc - ARM: dts: stm32: Create separate pinmux for qspi cs pin
   in stm32mp15-pinctrl.dtsi
- a306d8962a24 - ARM: dts: stm32: Rename mdio0 to mdio
- 0a5ebb1f3367 - ARM: dts: stm32: Replace SAI format with dai-format DT
   property
- ccdab19738a6 - ARM: dts: stm32: add adc support to stm32mp13
- 022932ab55fd - ARM: dts: stm32: add adc pins muxing on stm32mp135f-dk
- ab2806ddad9d - ARM: dts: stm32: add dummy vdd_adc regulator on
   stm32mp135f-dk
- e46a180c060f - ARM: dts: stm32: add adc support on stm32mp135f-dk
- 9ebf215fbae1 - ARM: dts: stm32: add PWR fixed regulators on stm32mp131
- 16f4ff60519a - ARM: dts: stm32: add USBPHYC and dual USB HS PHY support
   on stm32mp131
- 4a47f0f3e936 - ARM: dts: stm32: add UBSH EHCI and OHCI support on
   stm32mp131
- 2a46bb66c47f - ARM: dts: stm32: add USB OTG HS support on stm32mp131
- 9ebf215fbae1 - ARM: dts: stm32: add fixed regulators to support usb on
   stm32mp135f-dk
- 16f4ff60519a - ARM: dts: stm32: enable USB HS phys on stm32mp135f-dk
- c4e7254cf6dc - ARM: dts: stm32: enable USB Host EHCI on stm32mp135f-dk
- 44978e135916 - ARM: dts: stm32: add pins for stm32g0 typec controller on 
stm32mp13
- 4f532403b1e5 - ARM: dts: stm32: enable USB OTG in dual role mode on
   stm32mp135f-dk
- e1f15571c96c - ARM: dts: stm32: add mcp23017 pinctrl entry for stm32mp13
- 6cc71374002e - ARM: dts: stm32: add mcp23017 IO expander on I2C1 on
   stm32mp135f-dk
- 7ffd2266bd32 - ARM: dts: stm32: Fix qspi pinctrl phandle for
   stm32mp15xx-dhcor-som
- 21d83512bf2b - ARM: dts: stm32: Fix qspi pinctrl phandle for
   stm32mp15xx-dhcom-som
- 732dbcf52f74 - ARM: dts: stm32: Fix qspi pinctrl phandle for
   stm32mp151a-prtt1l
- 003b7c6b24f4 - ARM: dts: stm32: remove sai kernel clock on
   stm32mp15xx-dkx
- f2b17b39bfff - ARM: dts: stm32: rename sound card on stm32mp15xx-dkx
- dee3cb759d3d - ARM: dts: stm32: Remove the pins-are-numbered property
- ae8cf3b48727 - ARM: dts: stm32: add i2s nodes on stm32mp131
- 619746a27bd0 - ARM: dts: stm32: add sai nodes on stm32mp131
- c5e05d08ef90 - ARM: dts: stm32: add spdifrx node on stm32mp131
- 0a5afd3ee0d0 - ARM: dts: stm32: add dfsdm node on stm32mp131
- bf9d876bea2e - ARM: dts: stm32: add timers support on stm32mp131
- a3183748371d - ARM: dts: stm32: add timer pins muxing for stm32mp135f-dk
- a9060c1326bc - ARM: dts: stm32: add timers support on stm32mp135f-dk
- a12154058f75 - ARM: dts: stm32: Fix User button on stm32mp135f-dk
- 2f33df889e99 - ARM: dts: stm32: Use new media bus type macros
- 366384e49551 - ARM: dts: stm32: Update part number NVMEM description on
   stm32mp131


Thanks.

Reviewed-by: Marek Vasut 

You really could've just included the one Linux 6.3 commit hash and 
wrote this is synced with Linux 6.3, since all the commits listed above 
would be part of the Linux 6.3 commit hash anyway . It would make the 
commit message shorter, but this variant is also fine.


[PATCH v2 30/30] ide: Make use of U-Boot types

2023-04-25 Thread Simon Glass
Use standard U-Boot types in the file to make the code less verbose.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Rebase to master

 drivers/block/ide.c | 79 +
 1 file changed, 37 insertions(+), 42 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index af0c951eb89..89201dd4d22 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -63,7 +63,7 @@ static void ide_reset(void)
}
 }
 
-static void ide_outb(int dev, int port, unsigned char val)
+static void ide_outb(int dev, int port, u8 val)
 {
log_debug("(dev= %d, port= %#x, val= 0x%02x) : @ 0x%08lx\n",
  dev, port, val, ATA_CURR_BASE(dev) + port);
@@ -71,7 +71,7 @@ static void ide_outb(int dev, int port, unsigned char val)
outb(val, ATA_CURR_BASE(dev) + port);
 }
 
-static unsigned char ide_inb(int dev, int port)
+static u8 ide_inb(int dev, int port)
 {
uchar val;
 
@@ -119,10 +119,9 @@ static uchar ide_wait(int dev, ulong t)
  * terminate the string
  * "len" is the size of available memory including the terminating '\0'
  */
-static void ident_cpy(unsigned char *dst, unsigned char *src,
- unsigned int len)
+static void ident_cpy(u8 *dst, u8 *src, uint len)
 {
-   unsigned char *end, *last;
+   u8 *end, *last;
 
last = dst;
end = src + len - 1;
@@ -209,10 +208,9 @@ static uchar atapi_wait_mask(int dev, ulong t, uchar mask, 
uchar res)
 /*
  * issue an atapi command
  */
-static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
-unsigned char *buffer, int buflen)
+static u8 atapi_issue(int device, u8 *ccb, int ccblen, u8 *buffer, int buflen)
 {
-   unsigned char c, err, mask, res;
+   u8 c, err, mask, res;
int n;
 
/* Select device
@@ -231,8 +229,8 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
ide_outb(device, ATA_ERROR_REG, 0); /* no DMA, no overlaped */
ide_outb(device, ATA_SECT_CNT, 0);
ide_outb(device, ATA_SECT_NUM, 0);
-   ide_outb(device, ATA_CYL_LOW, (unsigned char)(buflen & 0xff));
-   ide_outb(device, ATA_CYL_HIGH, (unsigned char)((buflen >> 8) & 0xff));
+   ide_outb(device, ATA_CYL_LOW, (u8)(buflen & 0xff));
+   ide_outb(device, ATA_CYL_HIGH, (u8)((buflen >> 8) & 0xff));
ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
 
ide_outb(device, ATA_COMMAND, ATA_CMD_PACKET);
@@ -250,7 +248,7 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
}
 
/* write command block */
-   ide_output_data_shorts(device, (unsigned short *)ccb, ccblen / 2);
+   ide_output_data_shorts(device, (ushort *)ccb, ccblen / 2);
 
/* ATAPI Command written wait for completition */
mdelay(5);  /* device must set bsy */
@@ -301,13 +299,11 @@ static unsigned char atapi_issue(int device, unsigned 
char *ccb, int ccblen,
/* ok now decide if it is an in or output */
if (!(ide_inb(device, ATA_SECT_CNT) & 0x02)) {
log_debug("Write to device\n");
-   ide_output_data_shorts(device, (unsigned short *)buffer,
-  n);
+   ide_output_data_shorts(device, (ushort *)buffer, n);
} else {
log_debug("Read from device @ %p shorts %d\n", buffer,
  n);
-   ide_input_data_shorts(device, (unsigned short *)buffer,
- n);
+   ide_input_data_shorts(device, (ushort *)buffer, n);
}
}
mdelay(5);  /* seems that some CD ROMs need this... */
@@ -332,12 +328,11 @@ AI_OUT:
 #define ATAPI_DRIVE_NOT_READY  100
 #define ATAPI_UNIT_ATTN10
 
-static unsigned char atapi_issue_autoreq(int device, unsigned char *ccb,
-int ccblen,
-unsigned char *buffer, int buflen)
+static u8 atapi_issue_autoreq(int device, u8 *ccb, int ccblen, u8 *buffer,
+ int buflen)
 {
-   unsigned char sense_data[18], sense_ccb[12];
-   unsigned char res, key, asc, ascq;
+   u8 sense_data[18], sense_ccb[12];
+   u8 res, key, asc, ascq;
int notready, unitattn;
 
unitattn = ATAPI_UNIT_ATTN;
@@ -415,7 +410,7 @@ static ulong atapi_read(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
struct blk_desc *desc = dev_get_uclass_plat(dev);
int device = desc->devnum;
ulong n = 0;
-   unsigned char ccb[12];  /* Command descriptor block */
+   u8 ccb[12]; /* Command descriptor block */
ulong cnt;
 
log_debug("%d start " LBAF " blocks " LBAF " buffer at %lx\n", device,
@@ -429,19 +424,19 @@ static ulong 

[PATCH v2 22/30] ide: Use desc consistently for struct blk_desc

2023-04-25 Thread Simon Glass
Most of the code uses 'desc' as the variable name for a blk descriptor.
Change ide to do the same.

Tidy up some extra brackets and types while we are here.

Leave the code in ide_probe() alone since it is about to be refactored.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 101 +---
 1 file changed, 47 insertions(+), 54 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index d682d6ad9e6..835e781fccb 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -414,8 +414,8 @@ error:
 static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
void *buffer)
 {
-   struct blk_desc *block_dev = dev_get_uclass_plat(dev);
-   int device = block_dev->devnum;
+   struct blk_desc *desc = dev_get_uclass_plat(dev);
+   int device = desc->devnum;
ulong n = 0;
unsigned char ccb[12];  /* Command descriptor block */
ulong cnt;
@@ -456,15 +456,15 @@ static ulong atapi_read(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
return n;
 }
 
-static void atapi_inquiry(struct blk_desc *dev_desc)
+static void atapi_inquiry(struct blk_desc *desc)
 {
unsigned char ccb[12];  /* Command descriptor block */
unsigned char iobuf[64];/* temp buf */
unsigned char c;
int device;
 
-   device = dev_desc->devnum;
-   dev_desc->type = DEV_TYPE_UNKNOWN;  /* not yet valid */
+   device = desc->devnum;
+   desc->type = DEV_TYPE_UNKNOWN;  /* not yet valid */
 
memset(ccb, 0, sizeof(ccb));
memset(iobuf, 0, sizeof(iobuf));
@@ -478,20 +478,20 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
return;
 
/* copy device ident strings */
-   ident_cpy((unsigned char *)dev_desc->vendor, [8], 8);
-   ident_cpy((unsigned char *)dev_desc->product, [16], 16);
-   ident_cpy((unsigned char *)dev_desc->revision, [32], 5);
+   ident_cpy((u8 *)desc->vendor, [8], 8);
+   ident_cpy((u8 *)desc->product, [16], 16);
+   ident_cpy((u8 *)desc->revision, [32], 5);
 
-   dev_desc->lun = 0;
-   dev_desc->lba = 0;
-   dev_desc->blksz = 0;
-   dev_desc->log2blksz = LOG2_INVALID(typeof(dev_desc->log2blksz));
-   dev_desc->type = iobuf[0] & 0x1f;
+   desc->lun = 0;
+   desc->lba = 0;
+   desc->blksz = 0;
+   desc->log2blksz = LOG2_INVALID(typeof(desc->log2blksz));
+   desc->type = iobuf[0] & 0x1f;
 
if ((iobuf[1] & 0x80) == 0x80)
-   dev_desc->removable = 1;
+   desc->removable = 1;
else
-   dev_desc->removable = 0;
+   desc->removable = 0;
 
memset(ccb, 0, sizeof(ccb));
memset(iobuf, 0, sizeof(iobuf));
@@ -524,19 +524,17 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
  iobuf[0], iobuf[1], iobuf[2], iobuf[3],
  iobuf[4], iobuf[5], iobuf[6], iobuf[7]);
 
-   dev_desc->lba = ((unsigned long) iobuf[0] << 24) +
-   ((unsigned long) iobuf[1] << 16) +
-   ((unsigned long) iobuf[2] << 8) + ((unsigned long) iobuf[3]);
-   dev_desc->blksz = ((unsigned long) iobuf[4] << 24) +
-   ((unsigned long) iobuf[5] << 16) +
-   ((unsigned long) iobuf[6] << 8) + ((unsigned long) iobuf[7]);
-   dev_desc->log2blksz = LOG2(dev_desc->blksz);
+   desc->lba = (ulong)iobuf[0] << 24 | (ulong)iobuf[1] << 16 |
+   (ulong)iobuf[2] << 8 | (ulong)iobuf[3];
+   desc->blksz = (ulong)iobuf[4] << 24 | (ulong)iobuf[5] << 16 |
+   (ulong)iobuf[6] << 8 | (ulong)iobuf[7];
+   desc->log2blksz = LOG2(desc->blksz);
 
/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */
-   dev_desc->lba48 = false;
+   desc->lba48 = false;
 }
 
-static void ide_ident(struct blk_desc *dev_desc)
+static void ide_ident(struct blk_desc *desc)
 {
unsigned char c;
hd_driveid_t iop;
@@ -544,13 +542,13 @@ static void ide_ident(struct blk_desc *dev_desc)
int tries = 1;
int device;
 
-   device = dev_desc->devnum;
+   device = desc->devnum;
printf("  Device %d: ", device);
 
/* Select device
 */
ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
-   dev_desc->uclass_id = UCLASS_IDE;
+   desc->uclass_id = UCLASS_IDE;
if (IS_ENABLED(CONFIG_ATAPI))
tries = 2;
 
@@ -610,49 +608,44 @@ static void ide_ident(struct blk_desc *dev_desc)
 
ide_input_swap_data(device, (ulong *), ATA_SECTORWORDS);
 
-   ident_cpy((unsigned char *)dev_desc->revision, iop.fw_rev,
- sizeof(dev_desc->revision));
-   ident_cpy((unsigned char *)dev_desc->vendor, iop.model,
- sizeof(dev_desc->vendor));
-   ident_cpy((unsigned char *)dev_desc->product, iop.serial_no,
- sizeof(dev_desc->product));
+   ident_cpy((u8 

[PATCH v2 24/30] ide: Move all blk_desc init into ide_ident()

2023-04-25 Thread Simon Glass
Rather than having the caller fill some of this in, do it all in the
ide_ident() function, since it knows all the values.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 16b119ecbe1..b1c897d6a41 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -548,13 +548,16 @@ static int ide_ident(int device, struct blk_desc *desc)
bool is_atapi = false;
int tries = 1;
 
+   memset(desc, '\0', sizeof(*desc));
desc->devnum = device;
+   desc->type = DEV_TYPE_UNKNOWN;
+   desc->uclass_id = UCLASS_IDE;
+   desc->log2blksz = LOG2_INVALID(typeof(desc->log2blksz));
printf("  Device %d: ", device);
 
/* Select device
 */
ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
-   desc->uclass_id = UCLASS_IDE;
if (IS_ENABLED(CONFIG_ATAPI))
tries = 2;
 
@@ -1035,13 +1038,6 @@ static int ide_probe(struct udevice *udev)
if (!bus_ok[IDE_BUS(i)])
continue;
 
-   ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
-   ide_dev_desc[i].uclass_id = UCLASS_IDE;
-   ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
-   ide_dev_desc[i].blksz = 0;
-   ide_dev_desc[i].log2blksz =
-   LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
-   ide_dev_desc[i].lba = 0;
ret = ide_ident(i, _dev_desc[i]);
dev_print(_dev_desc[i]);
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 29/30] ide: Simplify expressions and hex values

2023-04-25 Thread Simon Glass
The code has quite a few unnecessary brackets and comparisons to zero,
etc. Fix these up as well as some upper-case hex values and use of 0x in
printf() strings.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 115 +---
 1 file changed, 56 insertions(+), 59 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 37236f6058b..af0c951eb89 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -108,7 +108,7 @@ static uchar ide_wait(int dev, ulong t)
 
while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) {
udelay(100);
-   if (delay-- == 0)
+   if (!delay--)
break;
}
return c;
@@ -153,7 +153,7 @@ OUT:
  * we have our own transfer functions, 2 bytes alligned */
 static void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
-   uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
+   uintptr_t paddr = ATA_CURR_BASE(dev) + ATA_DATA_REG;
ushort *dbuf;
 
dbuf = (ushort *)sect_buf;
@@ -168,7 +168,7 @@ static void ide_output_data_shorts(int dev, ushort 
*sect_buf, int shorts)
 
 static void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
-   uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
+   uintptr_t paddr = ATA_CURR_BASE(dev) + ATA_DATA_REG;
ushort *dbuf;
 
dbuf = (ushort *)sect_buf;
@@ -195,12 +195,12 @@ static uchar atapi_wait_mask(int dev, ulong t, uchar 
mask, uchar res)
/* prevents to read the status before valid */
c = ide_inb(dev, ATA_DEV_CTL);
 
-   while (((c = ide_inb(dev, ATA_STATUS)) & mask) != res) {
+   while (c = ide_inb(dev, ATA_STATUS) & mask, c != res) {
/* break if error occurs (doesn't make sense to wait more) */
if ((c & ATA_STAT_ERR) == ATA_STAT_ERR)
break;
udelay(100);
-   if (delay-- == 0)
+   if (!delay--)
break;
}
return c;
@@ -224,16 +224,15 @@ static unsigned char atapi_issue(int device, unsigned 
char *ccb, int ccblen,
if ((c & mask) != res) {
printf("ATAPI_ISSUE: device %d not ready status %x\n", device,
   c);
-   err = 0xFF;
+   err = 0xff;
goto AI_OUT;
}
/* write taskfile */
ide_outb(device, ATA_ERROR_REG, 0); /* no DMA, no overlaped */
ide_outb(device, ATA_SECT_CNT, 0);
ide_outb(device, ATA_SECT_NUM, 0);
-   ide_outb(device, ATA_CYL_LOW, (unsigned char) (buflen & 0xFF));
-   ide_outb(device, ATA_CYL_HIGH,
-(unsigned char) ((buflen >> 8) & 0xFF));
+   ide_outb(device, ATA_CYL_LOW, (unsigned char)(buflen & 0xff));
+   ide_outb(device, ATA_CYL_HIGH, (unsigned char)((buflen >> 8) & 0xff));
ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
 
ide_outb(device, ATA_COMMAND, ATA_CMD_PACKET);
@@ -244,9 +243,9 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
 
if ((c & mask) != res) {/* DRQ must be 1, BSY 0 */
-   printf("ATAPI_ISSUE: Error (no IRQ) before sending ccb dev %d 
status 0x%02x\n",
+   printf("ATAPI_ISSUE: Error (no IRQ) before sending ccb dev %d 
status %#02x\n",
   device, c);
-   err = 0xFF;
+   err = 0xff;
goto AI_OUT;
}
 
@@ -271,9 +270,9 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
log_debug("1 returned sense key %x status %02x\n",
  err, c);
} else {
-   printf("ATAPI_ISSUE: (no DRQ) after sending ccb (%x)  
status 0x%02x\n",
+   printf("ATAPI_ISSUE: (no DRQ) after sending ccb (%x)  
status %#02x\n",
   ccb[0], c);
-   err = 0xFF;
+   err = 0xff;
}
goto AI_OUT;
}
@@ -286,7 +285,7 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
err = 0xff;
goto AI_OUT;
}
-   if ((n == 0) && (buflen < 0)) {
+   if (!n && buflen < 0) {
printf("ERROR, transfer bytes %d requested %d\n", n, buflen);
err = 0xff;
goto AI_OUT;
@@ -295,12 +294,12 @@ static unsigned char atapi_issue(int device, unsigned 
char *ccb, int ccblen,
log_debug("WARNING, transfer bytes %d not equal with requested 
%d\n",
  n, buflen);
}
-   if (n != 0) {   /* data transfer */
+   if (n) {/* data transfer */
log_debug("ATAPI_ISSUE: %d Bytes to 

[PATCH v2 28/30] ide: Convert to use log_debug()

2023-04-25 Thread Simon Glass
Avoid the use of the function name in a few of the debug() calls, since
this causes a checkpatch warning. Convert all other calls too.

Use lower-case hex consistently.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 80 ++---
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index fb409338783..37236f6058b 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -65,8 +65,8 @@ static void ide_reset(void)
 
 static void ide_outb(int dev, int port, unsigned char val)
 {
-   debug("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
- dev, port, val, ATA_CURR_BASE(dev) + port);
+   log_debug("(dev= %d, port= %#x, val= 0x%02x) : @ 0x%08lx\n",
+ dev, port, val, ATA_CURR_BASE(dev) + port);
 
outb(val, ATA_CURR_BASE(dev) + port);
 }
@@ -77,8 +77,8 @@ static unsigned char ide_inb(int dev, int port)
 
val = inb(ATA_CURR_BASE(dev) + port);
 
-   debug("ide_inb (dev= %d, port= 0x%x) : @ 0x%08lx -> 0x%02x\n",
- dev, port, ATA_CURR_BASE(dev) + port, val);
+   log_debug("(dev= %d, port= %#x) : @ 0x%08lx -> 0x%02x\n",
+ dev, port, ATA_CURR_BASE(dev) + port, val);
return val;
 }
 
@@ -87,7 +87,7 @@ static void ide_input_swap_data(int dev, ulong *sect_buf, int 
words)
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ushort *dbuf = (ushort *)sect_buf;
 
-   debug("in input swap data base for read is %p\n", (void *)paddr);
+   log_debug("in input swap data base for read is %p\n", (void *)paddr);
 
while (words--) {
EIEIO;
@@ -158,7 +158,7 @@ static void ide_output_data_shorts(int dev, ushort 
*sect_buf, int shorts)
 
dbuf = (ushort *)sect_buf;
 
-   debug("in output data shorts base for read is %p\n", (void *)paddr);
+   log_debug("in output data shorts base for read is %p\n", (void *)paddr);
 
while (shorts--) {
EIEIO;
@@ -173,7 +173,7 @@ static void ide_input_data_shorts(int dev, ushort 
*sect_buf, int shorts)
 
dbuf = (ushort *)sect_buf;
 
-   debug("in input data shorts base for read is %p\n", (void *)paddr);
+   log_debug("in input data shorts base for read is %p\n", (void *)paddr);
 
while (shorts--) {
EIEIO;
@@ -222,7 +222,7 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
if ((c & mask) != res) {
-   printf("ATAPI_ISSUE: device %d not ready status %X\n", device,
+   printf("ATAPI_ISSUE: device %d not ready status %x\n", device,
   c);
err = 0xFF;
goto AI_OUT;
@@ -268,8 +268,8 @@ static unsigned char atapi_issue(int device, unsigned char 
*ccb, int ccblen,
if ((c & mask) != res) {
if (c & ATA_STAT_ERR) {
err = (ide_inb(device, ATA_ERROR_REG)) >> 4;
-   debug("atapi_issue 1 returned sense key %X status 
%02X\n",
- err, c);
+   log_debug("1 returned sense key %x status %02x\n",
+ err, c);
} else {
printf("ATAPI_ISSUE: (no DRQ) after sending ccb (%x)  
status 0x%02x\n",
   ccb[0], c);
@@ -292,20 +292,21 @@ static unsigned char atapi_issue(int device, unsigned 
char *ccb, int ccblen,
goto AI_OUT;
}
if (n != buflen) {
-   debug("WARNING, transfer bytes %d not equal with requested 
%d\n",
- n, buflen);
+   log_debug("WARNING, transfer bytes %d not equal with requested 
%d\n",
+ n, buflen);
}
if (n != 0) {   /* data transfer */
-   debug("ATAPI_ISSUE: %d Bytes to transfer\n", n);
+   log_debug("ATAPI_ISSUE: %d Bytes to transfer\n", n);
/* we transfer shorts */
n >>= 1;
/* ok now decide if it is an in or output */
if ((ide_inb(device, ATA_SECT_CNT) & 0x02) == 0) {
-   debug("Write to device\n");
+   log_debug("Write to device\n");
ide_output_data_shorts(device, (unsigned short *)buffer,
   n);
} else {
-   debug("Read from device @ %p shorts %d\n", buffer, n);
+   log_debug("Read from device @ %p shorts %d\n", buffer,
+ n);
ide_input_data_shorts(device, (unsigned short *)buffer,
  n);
}
@@ -316,8 +317,7 @@ static unsigned 

[PATCH v2 27/30] ide: Tidy up ide_reset()

2023-04-25 Thread Simon Glass
Avoid using #ifdef and use a single function declaration, so it is easier
to read.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 72216540d04..fb409338783 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -45,24 +45,23 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 
 #define IDE_SPIN_UP_TIME_OUT 5000 /* 5 sec spin-up timeout */
 
-#ifdef CONFIG_IDE_RESET
 static void ide_reset(void)
 {
-   ide_set_reset(1);   /* assert reset */
+   if (IS_ENABLED(CONFIG_IDE_RESET)) {
+   /* assert reset */
+   ide_set_reset(1);
 
-   /* the reset signal shall be asserted for et least 25 us */
-   udelay(25);
+   /* the reset signal shall be asserted for et least 25 us */
+   udelay(25);
 
-   schedule();
+   schedule();
 
-   /* de-assert RESET signal */
-   ide_set_reset(0);
+   /* de-assert RESET signal */
+   ide_set_reset(0);
 
-   mdelay(250);
+   mdelay(250);
+   }
 }
-#else
-#define ide_reset()/* dummy */
-#endif /* CONFIG_IDE_RESET */
 
 static void ide_outb(int dev, int port, unsigned char val)
 {
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 25/30] ide: Use a single local blk_desc for ide_ident()

2023-04-25 Thread Simon Glass
We only use one member of the ide_dev_desc[] array at a time and it does
not stick around outside ide_probe(). Use a single element instead.

Copy over the missing members of blk_desc at the same, since this was
missing from the previous code.

Signed-off-by: Simon Glass 
Fixes: 68e6f221ed0 ("block: ide: Fix block read/write with driver model")
---

(no changes since v1)

 drivers/block/ide.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index b1c897d6a41..4c2a6a8e530 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1005,7 +1005,6 @@ BOOTDEV_HUNTER(ide_bootdev_hunter) = {
 
 static int ide_probe(struct udevice *udev)
 {
-   struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
int i, bus;
 
@@ -1028,7 +1027,7 @@ static int ide_probe(struct udevice *udev)
schedule();
 
for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
-   struct blk_desc *desc;
+   struct blk_desc *desc, pdesc;
struct udevice *blk;
lbaint_t size;
char name[20];
@@ -1038,16 +1037,16 @@ static int ide_probe(struct udevice *udev)
if (!bus_ok[IDE_BUS(i)])
continue;
 
-   ret = ide_ident(i, _dev_desc[i]);
-   dev_print(_dev_desc[i]);
+   ret = ide_ident(i, );
+   dev_print();
 
if (ret)
continue;
 
sprintf(name, "blk#%d", i);
 
-   blksz = ide_dev_desc[i].blksz;
-   size = blksz * ide_dev_desc[i].lba;
+   blksz = pdesc.blksz;
+   size = blksz * pdesc.lba;
 
/*
 * With CDROM, if there is no CD inserted, blksz will
@@ -1066,12 +1065,13 @@ static int ide_probe(struct udevice *udev)
 
/* fill in device vendor/product/rev strings */
desc = dev_get_uclass_plat(blk);
-   strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
-   BLK_VEN_SIZE);
-   strlcpy(desc->product, ide_dev_desc[desc->devnum].product,
-   BLK_PRD_SIZE);
-   strlcpy(desc->revision, ide_dev_desc[desc->devnum].revision,
-   BLK_REV_SIZE);
+   strlcpy(desc->vendor, pdesc.vendor, BLK_VEN_SIZE);
+   strlcpy(desc->product, pdesc.product, BLK_PRD_SIZE);
+   strlcpy(desc->revision, pdesc.revision, BLK_REV_SIZE);
+   desc->removable = pdesc.removable;
+   desc->atapi = pdesc.atapi;
+   desc->lba48 = pdesc.lba48;
+   desc->type = pdesc.type;
 
ret = bootdev_setup_for_dev(udev, "ide_bootdev");
if (ret)
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 26/30] ide: Correct LBA setting

2023-04-25 Thread Simon Glass
Fix a longstanding bug where the LBA is calculated as the size of the
media instead of the number of blocks. This was perhaps not noticed
earlier since it prints the correct value first, before setting the wrong
value.

Drop the unnecessary blksz variable while we are here.

Signed-off-by: Simon Glass 
Fixes: 68e6f221ed0 ("block: ide: Fix block read/write with driver model")
---

(no changes since v1)

 drivers/block/ide.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 4c2a6a8e530..72216540d04 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1029,9 +1029,7 @@ static int ide_probe(struct udevice *udev)
for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
struct blk_desc *desc, pdesc;
struct udevice *blk;
-   lbaint_t size;
char name[20];
-   int blksz;
int ret;
 
if (!bus_ok[IDE_BUS(i)])
@@ -1045,17 +1043,14 @@ static int ide_probe(struct udevice *udev)
 
sprintf(name, "blk#%d", i);
 
-   blksz = pdesc.blksz;
-   size = blksz * pdesc.lba;
-
/*
 * With CDROM, if there is no CD inserted, blksz will
 * be zero, don't bother to create IDE block device.
 */
-   if (!blksz)
+   if (!pdesc.blksz)
continue;
ret = blk_create_devicef(udev, "ide_blk", name, UCLASS_IDE, i,
-blksz, size, );
+pdesc.blksz, pdesc.lba, );
if (ret)
return ret;
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 23/30] ide: Make ide_ident() return an error code

2023-04-25 Thread Simon Glass
Update ide_ident() to indicate whether it finds a device or not. Use
that to decide whether to create a block device for it, rather than
looking DEV_TYPE_UNKNOWN.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 101 +++-
 1 file changed, 53 insertions(+), 48 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 835e781fccb..16b119ecbe1 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -534,15 +534,21 @@ static void atapi_inquiry(struct blk_desc *desc)
desc->lba48 = false;
 }
 
-static void ide_ident(struct blk_desc *desc)
+/**
+ * ide_ident() - Identify an IDE device
+ *
+ * @device: Device number to use
+ * @desc: Block descriptor to fill in
+ * Returns: 0 if OK, -ENOENT if no device is found
+ */
+static int ide_ident(int device, struct blk_desc *desc)
 {
unsigned char c;
hd_driveid_t iop;
bool is_atapi = false;
int tries = 1;
-   int device;
 
-   device = desc->devnum;
+   desc->devnum = device;
printf("  Device %d: ", device);
 
/* Select device
@@ -604,7 +610,7 @@ static void ide_ident(struct blk_desc *desc)
}
 
if (!tries) /* Not found */
-   return;
+   return -ENOENT;
 
ide_input_swap_data(device, (ulong *), ATA_SECTORWORDS);
 
@@ -620,7 +626,7 @@ static void ide_ident(struct blk_desc *desc)
if (IS_ENABLED(CONFIG_ATAPI) && is_atapi) {
desc->atapi = true;
atapi_inquiry(desc);
-   return;
+   return 0;
}
 
iop.lba_capacity[0] = be16_to_cpu(iop.lba_capacity[0]);
@@ -661,6 +667,8 @@ static void ide_ident(struct blk_desc *desc)
udelay(50);
c = ide_wait(device, IDE_TIME_OUT); /* can't take over 500 ms */
 #endif
+
+   return 0;
 }
 
 /**
@@ -1017,64 +1025,61 @@ static int ide_probe(struct udevice *udev)
schedule();
 
for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
+   struct blk_desc *desc;
+   struct udevice *blk;
+   lbaint_t size;
+   char name[20];
+   int blksz;
+   int ret;
+
if (!bus_ok[IDE_BUS(i)])
continue;
 
ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
ide_dev_desc[i].uclass_id = UCLASS_IDE;
-   ide_dev_desc[i].devnum = i;
ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
ide_dev_desc[i].blksz = 0;
ide_dev_desc[i].log2blksz =
LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
ide_dev_desc[i].lba = 0;
-   ide_ident(_dev_desc[i]);
+   ret = ide_ident(i, _dev_desc[i]);
dev_print(_dev_desc[i]);
 
-   if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
-   struct udevice *blk_dev;
-   struct blk_desc *desc;
-   lbaint_t size;
-   char name[20];
-   int blksz;
-   int ret;
+   if (ret)
+   continue;
 
-   sprintf(name, "blk#%d", i);
+   sprintf(name, "blk#%d", i);
 
-   blksz = ide_dev_desc[i].blksz;
-   size = blksz * ide_dev_desc[i].lba;
+   blksz = ide_dev_desc[i].blksz;
+   size = blksz * ide_dev_desc[i].lba;
 
-   /*
-* With CDROM, if there is no CD inserted, blksz will
-* be zero, don't bother to create IDE block device.
-*/
-   if (!blksz)
-   continue;
-   ret = blk_create_devicef(udev, "ide_blk", name,
-UCLASS_IDE, i,
-blksz, size, _dev);
-   if (ret)
-   return ret;
-
-   ret = blk_probe_or_unbind(blk_dev);
-   if (ret)
-   return ret;
-
-   /* fill in device vendor/product/rev strings */
-   desc = dev_get_uclass_plat(blk_dev);
-   strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
-   BLK_VEN_SIZE);
-   strlcpy(desc->product,
-   ide_dev_desc[desc->devnum].product,
-   BLK_PRD_SIZE);
-   strlcpy(desc->revision,
-   ide_dev_desc[desc->devnum].revision,
-   BLK_REV_SIZE);
-
-   ret = bootdev_setup_for_dev(udev, "ide_bootdev");
-   if (ret)
-   return 

[PATCH v2 20/30] ide: Move ide_init() entirely within ide_probe()

2023-04-25 Thread Simon Glass
Now that ide_probe() is the only caller of ide_init(), move all the code
into the probe function, so it is easier to refactor it.

Move ide_dev_desc[] into ide_probe() to, since it is the only user.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 84 -
 1 file changed, 38 insertions(+), 46 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index ecac8b6cfd5..5fbf144da9d 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -39,8 +39,6 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 #define ATA_CURR_BASE(dev) (CONFIG_SYS_ATA_BASE_ADDR + \
ide_bus_offset[IDE_BUS(dev)])
 
-struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
-
 #define IDE_TIME_OUT   2000/* 2 sec timeout */
 
 #define ATAPI_TIME_OUT 7000/* 7 sec timeout (5 sec seems to work...) */
@@ -719,44 +717,6 @@ static int ide_init_one(int bus)
return 0;
 }
 
-static void ide_init(void)
-{
-   bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
-   int i, bus;
-
-   schedule();
-
-   /* ATAPI Drives seems to need a proper IDE Reset */
-   ide_reset();
-
-   /*
-* Wait for IDE to get ready.
-* According to spec, this can take up to 31 seconds!
-*/
-   for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
-   bus_ok[bus] = !ide_init_one(bus);
-   schedule();
-   }
-
-   putc('\n');
-
-   for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) {
-   ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
-   ide_dev_desc[i].uclass_id = UCLASS_IDE;
-   ide_dev_desc[i].devnum = i;
-   ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
-   ide_dev_desc[i].blksz = 0;
-   ide_dev_desc[i].log2blksz =
-   LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
-   ide_dev_desc[i].lba = 0;
-   if (!bus_ok[IDE_BUS(i)])
-   continue;
-   ide_ident(_dev_desc[i]);
-   dev_print(_dev_desc[i]);
-   }
-   schedule();
-}
-
 static void ide_output_data(int dev, const ulong *sect_buf, int words)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
@@ -1041,9 +1001,41 @@ BOOTDEV_HUNTER(ide_bootdev_hunter) = {
 
 static int ide_probe(struct udevice *udev)
 {
-   int i;
+   struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
+   bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
+   int i, bus;
+
+   schedule();
+
+   /* ATAPI Drives seems to need a proper IDE Reset */
+   ide_reset();
+
+   /*
+* Wait for IDE to get ready.
+* According to spec, this can take up to 31 seconds!
+*/
+   for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
+   bus_ok[bus] = !ide_init_one(bus);
+   schedule();
+   }
 
-   ide_init();
+   putc('\n');
+
+   for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) {
+   ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
+   ide_dev_desc[i].uclass_id = UCLASS_IDE;
+   ide_dev_desc[i].devnum = i;
+   ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
+   ide_dev_desc[i].blksz = 0;
+   ide_dev_desc[i].log2blksz =
+   LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
+   ide_dev_desc[i].lba = 0;
+   if (!bus_ok[IDE_BUS(i)])
+   continue;
+   ide_ident(_dev_desc[i]);
+   dev_print(_dev_desc[i]);
+   }
+   schedule();
 
for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
@@ -1075,10 +1067,6 @@ static int ide_probe(struct udevice *udev)
if (ret)
return ret;
 
-   ret = bootdev_setup_for_dev(udev, "ide_bootdev");
-   if (ret)
-   return log_msg_ret("bootdev", ret);
-
/* fill in device vendor/product/rev strings */
desc = dev_get_uclass_plat(blk_dev);
strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
@@ -1089,6 +1077,10 @@ static int ide_probe(struct udevice *udev)
strlcpy(desc->revision,
ide_dev_desc[desc->devnum].revision,
BLK_REV_SIZE);
+
+   ret = bootdev_setup_for_dev(udev, "ide_bootdev");
+   if (ret)
+   return log_msg_ret("bootdev", ret);
}
}
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 21/30] ide: Combine the two loops in ide_probe()

2023-04-25 Thread Simon Glass
The two loops in this function operate on the same ide_dev_desc[] array.
Combine them to reduce duplication.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 5fbf144da9d..d682d6ad9e6 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1021,7 +1021,12 @@ static int ide_probe(struct udevice *udev)
 
putc('\n');
 
-   for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) {
+   schedule();
+
+   for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
+   if (!bus_ok[IDE_BUS(i)])
+   continue;
+
ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
ide_dev_desc[i].uclass_id = UCLASS_IDE;
ide_dev_desc[i].devnum = i;
@@ -1030,14 +1035,9 @@ static int ide_probe(struct udevice *udev)
ide_dev_desc[i].log2blksz =
LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
ide_dev_desc[i].lba = 0;
-   if (!bus_ok[IDE_BUS(i)])
-   continue;
ide_ident(_dev_desc[i]);
dev_print(_dev_desc[i]);
-   }
-   schedule();
 
-   for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
struct udevice *blk_dev;
struct blk_desc *desc;
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 19/30] ide: Move setting of vendor strings into ide_probe()

2023-04-25 Thread Simon Glass
The current implementation adds this information in the block device's
probe() function, which is called in the blk_probe_or_unbind() in
ide_probe().

It is simpler to do this in ide_probe() itself, since the effect is the
same. This helps to consolidate use of ide_dev_desc[] which we would like
to remove.

Use strlcpy() to keep checkpatch happy.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 42 ++
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 36c726225d0..ecac8b6cfd5 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -987,24 +987,6 @@ ulong ide_or_atapi_read(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
return ide_read(dev, blknr, blkcnt, buffer);
 }
 
-static int ide_blk_probe(struct udevice *udev)
-{
-   struct blk_desc *desc = dev_get_uclass_plat(udev);
-
-   /* fill in device vendor/product/rev strings */
-   strncpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
-   BLK_VEN_SIZE);
-   desc->vendor[BLK_VEN_SIZE] = '\0';
-   strncpy(desc->product, ide_dev_desc[desc->devnum].product,
-   BLK_PRD_SIZE);
-   desc->product[BLK_PRD_SIZE] = '\0';
-   strncpy(desc->revision, ide_dev_desc[desc->devnum].revision,
-   BLK_REV_SIZE);
-   desc->revision[BLK_REV_SIZE] = '\0';
-
-   return 0;
-}
-
 static const struct blk_ops ide_blk_ops = {
.read   = ide_or_atapi_read,
.write  = ide_write,
@@ -1014,7 +996,6 @@ U_BOOT_DRIVER(ide_blk) = {
.name   = "ide_blk",
.id = UCLASS_BLK,
.ops= _blk_ops,
-   .probe  = ide_blk_probe,
 };
 
 static int ide_bootdev_bind(struct udevice *dev)
@@ -1060,17 +1041,19 @@ BOOTDEV_HUNTER(ide_bootdev_hunter) = {
 
 static int ide_probe(struct udevice *udev)
 {
-   struct udevice *blk_dev;
-   char name[20];
-   int blksz;
-   lbaint_t size;
int i;
-   int ret;
 
ide_init();
 
for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
+   struct udevice *blk_dev;
+   struct blk_desc *desc;
+   lbaint_t size;
+   char name[20];
+   int blksz;
+   int ret;
+
sprintf(name, "blk#%d", i);
 
blksz = ide_dev_desc[i].blksz;
@@ -1095,6 +1078,17 @@ static int ide_probe(struct udevice *udev)
ret = bootdev_setup_for_dev(udev, "ide_bootdev");
if (ret)
return log_msg_ret("bootdev", ret);
+
+   /* fill in device vendor/product/rev strings */
+   desc = dev_get_uclass_plat(blk_dev);
+   strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
+   BLK_VEN_SIZE);
+   strlcpy(desc->product,
+   ide_dev_desc[desc->devnum].product,
+   BLK_PRD_SIZE);
+   strlcpy(desc->revision,
+   ide_dev_desc[desc->devnum].revision,
+   BLK_REV_SIZE);
}
}
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 18/30] ide: Make ide_bus_ok a local variable

2023-04-25 Thread Simon Glass
This is only used in one place now, so make it a local variable.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Drop [] in patch subject

 drivers/block/ide.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index aac4462f57b..36c726225d0 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -39,8 +39,6 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 #define ATA_CURR_BASE(dev) (CONFIG_SYS_ATA_BASE_ADDR + \
ide_bus_offset[IDE_BUS(dev)])
 
-static int ide_bus_ok[CONFIG_SYS_IDE_MAXBUS];
-
 struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 
 #define IDE_TIME_OUT   2000/* 2 sec timeout */
@@ -52,11 +50,6 @@ struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 #ifdef CONFIG_IDE_RESET
 static void ide_reset(void)
 {
-   int i;
-
-   for (i = 0; i < CONFIG_SYS_IDE_MAXBUS; ++i)
-   ide_bus_ok[i] = 0;
-
ide_set_reset(1);   /* assert reset */
 
/* the reset signal shall be asserted for et least 25 us */
@@ -728,6 +721,7 @@ static int ide_init_one(int bus)
 
 static void ide_init(void)
 {
+   bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
int i, bus;
 
schedule();
@@ -740,7 +734,7 @@ static void ide_init(void)
 * According to spec, this can take up to 31 seconds!
 */
for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
-   ide_bus_ok[bus] = !ide_init_one(bus);
+   bus_ok[bus] = !ide_init_one(bus);
schedule();
}
 
@@ -755,7 +749,7 @@ static void ide_init(void)
ide_dev_desc[i].log2blksz =
LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
ide_dev_desc[i].lba = 0;
-   if (!ide_bus_ok[IDE_BUS(i)])
+   if (!bus_ok[IDE_BUS(i)])
continue;
ide_ident(_dev_desc[i]);
dev_print(_dev_desc[i]);
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 15/30] ide: Avoid preprocessor for CONFIG_ATAPI

2023-04-25 Thread Simon Glass
Use IS_ENABLED() instead for all conditions.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index a51a0008cae..6c5227a5c0e 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -465,8 +465,6 @@ static ulong atapi_read(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
return n;
 }
 
-#ifdef CONFIG_ATAPI
-
 static void atapi_inquiry(struct blk_desc *dev_desc)
 {
unsigned char ccb[12];  /* Command descriptor block */
@@ -549,8 +547,6 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
return;
 }
 
-#endif /* CONFIG_ATAPI */
-
 static void ide_ident(struct blk_desc *dev_desc)
 {
unsigned char c;
@@ -637,13 +633,11 @@ static void ide_ident(struct blk_desc *dev_desc)
else
dev_desc->removable = 0;
 
-#ifdef CONFIG_ATAPI
-   if (is_atapi) {
+   if (IS_ENABLED(CONFIG_ATAPI) && is_atapi) {
dev_desc->atapi = true;
atapi_inquiry(dev_desc);
return;
}
-#endif /* CONFIG_ATAPI */
 
iop.lba_capacity[0] = be16_to_cpu(iop.lba_capacity[0]);
iop.lba_capacity[1] = be16_to_cpu(iop.lba_capacity[1]);
@@ -732,11 +726,10 @@ static void ide_init(void)
if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
puts("not available  ");
debug("Status = 0x%02X ", c);
-#ifndef CONFIG_ATAPI   /* ATAPI Devices do not set DRDY */
-   } else if ((c & ATA_STAT_READY) == 0) {
+   } else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
+   /* ATAPI Devices do not set DRDY */
puts("not available  ");
debug("Status = 0x%02X ", c);
-#endif
} else {
puts("OK ");
ide_bus_ok[bus] = 1;
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 17/30] ide: Move bus init into a function

2023-04-25 Thread Simon Glass
Move this code into a separate function which returns whether the bus was
found, or not.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 87 +
 1 file changed, 48 insertions(+), 39 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 45201333c3c..aac4462f57b 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -679,9 +679,55 @@ static void ide_ident(struct blk_desc *dev_desc)
 #endif
 }
 
+/**
+ * ide_init_one() - Init one IDE device
+ *
+ * @bus: Bus to use
+ * Return: 0 iuf OK, -EIO if not available, -ETIMEDOUT if timed out
+ */
+static int ide_init_one(int bus)
+{
+   int dev = bus * CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS;
+   int i;
+   u8 c;
+
+   printf("Bus %d: ", bus);
+
+   /* Select device */
+   mdelay(100);
+   ide_outb(dev, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(dev));
+   mdelay(100);
+   i = 0;
+   do {
+   mdelay(10);
+
+   c = ide_inb(dev, ATA_STATUS);
+   i++;
+   if (i > (ATA_RESET_TIME * 100)) {
+   puts("** Timeout **\n");
+   return -ETIMEDOUT;
+   }
+   if (i >= 100 && !(i % 100))
+   putc('.');
+   } while (c & ATA_STAT_BUSY);
+
+   if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
+   puts("not available  ");
+   debug("Status = 0x%02X ", c);
+   return -EIO;
+   } else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
+   /* ATAPI Devices do not set DRDY */
+   puts("not available  ");
+   debug("Status = 0x%02X ", c);
+   return -EIO;
+   }
+   puts("OK ");
+
+   return 0;
+}
+
 static void ide_init(void)
 {
-   unsigned char c;
int i, bus;
 
schedule();
@@ -694,44 +740,7 @@ static void ide_init(void)
 * According to spec, this can take up to 31 seconds!
 */
for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
-   int dev =
-   bus * (CONFIG_SYS_IDE_MAXDEVICE /
-  CONFIG_SYS_IDE_MAXBUS);
-
-   printf("Bus %d: ", bus);
-
-   ide_bus_ok[bus] = 0;
-
-   /* Select device */
-   mdelay(100);
-   ide_outb(dev, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(dev));
-   mdelay(100);
-   i = 0;
-   do {
-   mdelay(10);
-
-   c = ide_inb(dev, ATA_STATUS);
-   i++;
-   if (i > (ATA_RESET_TIME * 100)) {
-   puts("** Timeout **\n");
-   return;
-   }
-   if ((i >= 100) && ((i % 100) == 0))
-   putc('.');
-
-   } while (c & ATA_STAT_BUSY);
-
-   if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
-   puts("not available  ");
-   debug("Status = 0x%02X ", c);
-   } else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
-   /* ATAPI Devices do not set DRDY */
-   puts("not available  ");
-   debug("Status = 0x%02X ", c);
-   } else {
-   puts("OK ");
-   ide_bus_ok[bus] = 1;
-   }
+   ide_bus_ok[bus] = !ide_init_one(bus);
schedule();
}
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 16/30] ide: Avoid preprocessor for CONFIG_LBA48

2023-04-25 Thread Simon Glass
Use IS_ENABLED() instead for all conditions. Add the 'lba48' flag into
struct blk_desc always, since it uses very little space. Use a bool so
the meaning is clearer.

Signed-off-by: Simon Glass 
Reviewed-by: Mattijs Korpershoek 
---

(no changes since v1)

 drivers/block/ide.c | 57 -
 include/blk.h   |  4 +---
 2 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 6c5227a5c0e..45201333c3c 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -540,11 +540,9 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
((unsigned long) iobuf[5] << 16) +
((unsigned long) iobuf[6] << 8) + ((unsigned long) iobuf[7]);
dev_desc->log2blksz = LOG2(dev_desc->blksz);
-#ifdef CONFIG_LBA48
+
/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */
-   dev_desc->lba48 = 0;
-#endif
-   return;
+   dev_desc->lba48 = false;
 }
 
 static void ide_ident(struct blk_desc *dev_desc)
@@ -645,9 +643,9 @@ static void ide_ident(struct blk_desc *dev_desc)
((unsigned long)iop.lba_capacity[0]) |
((unsigned long)iop.lba_capacity[1] << 16);
 
-#ifdef CONFIG_LBA48
-   if (iop.command_set_2 & 0x0400) {   /* LBA 48 support */
-   dev_desc->lba48 = 1;
+   if (IS_ENABLED(CONFIG_LBA48) && (iop.command_set_2 & 0x0400)) {
+   /* LBA 48 support */
+   dev_desc->lba48 = true;
for (int i = 0; i < 4; i++)
iop.lba48_capacity[i] = 
be16_to_cpu(iop.lba48_capacity[i]);
dev_desc->lba =
@@ -656,9 +654,9 @@ static void ide_ident(struct blk_desc *dev_desc)
((unsigned long long)iop.lba48_capacity[2] << 32) |
((unsigned long long)iop.lba48_capacity[3] << 48));
} else {
-   dev_desc->lba48 = 0;
+   dev_desc->lba48 = false;
}
-#endif /* CONFIG_LBA48 */
+
/* assuming HD */
dev_desc->type = DEV_TYPE_HARDDISK;
dev_desc->blksz = ATA_BLOCKSIZE;
@@ -795,15 +793,13 @@ static ulong ide_read(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
ulong n = 0;
unsigned char c;
unsigned char pwrsave = 0;  /* power save */
+   bool lba48 = false;
 
-#ifdef CONFIG_LBA48
-   unsigned char lba48 = 0;
-
-   if (blknr & 0xf000ULL) {
+   if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0xf000ULL)) {
/* more than 28 bits used, use 48bit mode */
-   lba48 = 1;
+   lba48 = true;
}
-#endif
+
debug("ide_read dev %d start " LBAF ", blocks " LBAF " buffer at %lX\n",
  device, blknr, blkcnt, (ulong) buffer);
 
@@ -845,8 +841,7 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, 
lbaint_t blkcnt,
printf("IDE read: device %d not ready\n", device);
break;
}
-#ifdef CONFIG_LBA48
-   if (lba48) {
+   if (IS_ENABLED(CONFIG_LBA48) && lba48) {
/* write high bits */
ide_outb(device, ATA_SECT_CNT, 0);
ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
@@ -858,21 +853,17 @@ static ulong ide_read(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
ide_outb(device, ATA_LBA_HIGH, 0);
 #endif
}
-#endif
ide_outb(device, ATA_SECT_CNT, 1);
ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
 
-#ifdef CONFIG_LBA48
-   if (lba48) {
+   if (IS_ENABLED(CONFIG_LBA48) && lba48) {
ide_outb(device, ATA_DEV_HD,
 ATA_LBA | ATA_DEVICE(device));
ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT);
 
-   } else
-#endif
-   {
+   } else {
ide_outb(device, ATA_DEV_HD, ATA_LBA |
 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ);
@@ -914,15 +905,12 @@ static ulong ide_write(struct udevice *dev, lbaint_t 
blknr, lbaint_t blkcnt,
int device = block_dev->devnum;
ulong n = 0;
unsigned char c;
+   bool lba48 = false;
 
-#ifdef CONFIG_LBA48
-   unsigned char lba48 = 0;
-
-   if (blknr & 0xf000ULL) {
+   if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0xf000ULL)) {
/* more than 28 bits used, use 48bit mode */
-   lba48 = 1;
+   lba48 = true;
}
-#endif
 
/* Select device
 */
@@ -935,8 +923,7 @@ static ulong 

[PATCH v2 14/30] ide: Simplify success condition

2023-04-25 Thread Simon Glass
Change the if() to remove extra brackets and check for the positive case
first, i.e. when a device is found. Exit the loop in that case, with the
retry logic in the 'else' part.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 36 +---
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 2f45bb6bffb..a51a0008cae 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -599,27 +599,25 @@ static void ide_ident(struct blk_desc *dev_desc)
c = ide_wait(device, IDE_TIME_OUT);
}
 
-   if (((c & ATA_STAT_DRQ) == 0) ||
-   ((c & (ATA_STAT_FAULT | ATA_STAT_ERR)) != 0)) {
-   if (IS_ENABLED(CONFIG_ATAPI)) {
-   /*
-* Need to soft reset the device
-* in case it's an ATAPI...
-*/
-   debug("Retrying...\n");
-   ide_outb(device, ATA_DEV_HD,
-ATA_LBA | ATA_DEVICE(device));
-   mdelay(100);
-   ide_outb(device, ATA_COMMAND, 0x08);
-   mdelay(500);
-   /* Select device */
-   ide_outb(device, ATA_DEV_HD,
-ATA_LBA | ATA_DEVICE(device));
-   }
-   tries--;
-   } else {
+   if ((c & ATA_STAT_DRQ) &&
+   !(c & (ATA_STAT_FAULT | ATA_STAT_ERR))) {
break;
+   } else if (IS_ENABLED(CONFIG_ATAPI)) {
+   /*
+* Need to soft reset the device
+* in case it's an ATAPI...
+*/
+   debug("Retrying...\n");
+   ide_outb(device, ATA_DEV_HD,
+ATA_LBA | ATA_DEVICE(device));
+   mdelay(100);
+   ide_outb(device, ATA_COMMAND, 0x08);
+   mdelay(500);
+   /* Select device */
+   ide_outb(device, ATA_DEV_HD,
+ATA_LBA | ATA_DEVICE(device));
}
+   tries--;
}
 
if (!tries) /* Not found */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 13/30] ide: Refactor confusing loop code

2023-04-25 Thread Simon Glass
This code is hard to follow as it uses #ifdef in a strange way. Adjust
it to avoid the preprocessor. Drop the special return for the non-ATAPI
case since we can rely on tries becoming 0 and exiting the loop.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 43 +++
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 782780fd302..2f45bb6bffb 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -555,10 +555,8 @@ static void ide_ident(struct blk_desc *dev_desc)
 {
unsigned char c;
hd_driveid_t iop;
-#ifdef CONFIG_ATAPI
bool is_atapi = false;
int tries = 1;
-#endif
int device;
 
device = dev_desc->devnum;
@@ -568,17 +566,16 @@ static void ide_ident(struct blk_desc *dev_desc)
 */
ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
dev_desc->uclass_id = UCLASS_IDE;
-#ifdef CONFIG_ATAPI
+   if (IS_ENABLED(CONFIG_ATAPI))
+   tries = 2;
 
-   tries = 2;
-
-   /* Warning: This will be tricky to read */
while (tries) {
/* check signature */
-   if ((ide_inb(device, ATA_SECT_CNT) == 0x01) &&
-   (ide_inb(device, ATA_SECT_NUM) == 0x01) &&
-   (ide_inb(device, ATA_CYL_LOW) == 0x14) &&
-   (ide_inb(device, ATA_CYL_HIGH) == 0xEB)) {
+   if (IS_ENABLED(CONFIG_ATAPI) &&
+   ide_inb(device, ATA_SECT_CNT) == 0x01 &&
+   ide_inb(device, ATA_SECT_NUM) == 0x01 &&
+   ide_inb(device, ATA_CYL_LOW) == 0x14 &&
+   ide_inb(device, ATA_CYL_HIGH) == 0xeb) {
/* ATAPI Signature found */
is_atapi = true;
/*
@@ -590,9 +587,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 * to become ready
 */
c = ide_wait(device, ATAPI_TIME_OUT);
-   } else
-#endif
-   {
+   } else {
/*
 * Start Ident Command
 */
@@ -606,8 +601,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 
if (((c & ATA_STAT_DRQ) == 0) ||
((c & (ATA_STAT_FAULT | ATA_STAT_ERR)) != 0)) {
-#ifdef CONFIG_ATAPI
-   {
+   if (IS_ENABLED(CONFIG_ATAPI)) {
/*
 * Need to soft reset the device
 * in case it's an ATAPI...
@@ -618,25 +612,18 @@ static void ide_ident(struct blk_desc *dev_desc)
mdelay(100);
ide_outb(device, ATA_COMMAND, 0x08);
mdelay(500);
+   /* Select device */
+   ide_outb(device, ATA_DEV_HD,
+ATA_LBA | ATA_DEVICE(device));
}
-   /*
-* Select device
-*/
-   ide_outb(device, ATA_DEV_HD,
-ATA_LBA | ATA_DEVICE(device));
tries--;
-#else
-   return;
-#endif
-   }
-#ifdef CONFIG_ATAPI
-   else
+   } else {
break;
-   }   /* see above - ugly to read */
+   }
+   }
 
if (!tries) /* Not found */
return;
-#endif
 
ide_input_swap_data(device, (ulong *), ATA_SECTORWORDS);
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 11/30] ide: Make function static

2023-04-25 Thread Simon Glass
Only one function is called from outside this file. Make all the others
static.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 23 +++
 include/ide.h   | 11 ---
 2 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 875192cba16..1d5e54d6eb9 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -219,8 +219,8 @@ static uchar atapi_wait_mask(int dev, ulong t, uchar mask, 
uchar res)
 /*
  * issue an atapi command
  */
-unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
- unsigned char *buffer, int buflen)
+static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
+unsigned char *buffer, int buflen)
 {
unsigned char c, err, mask, res;
int n;
@@ -343,10 +343,9 @@ AI_OUT:
 #define ATAPI_DRIVE_NOT_READY  100
 #define ATAPI_UNIT_ATTN10
 
-unsigned char atapi_issue_autoreq(int device,
- unsigned char *ccb,
- int ccblen,
- unsigned char *buffer, int buflen)
+static unsigned char atapi_issue_autoreq(int device, unsigned char *ccb,
+int ccblen,
+unsigned char *buffer, int buflen)
 {
unsigned char sense_data[18], sense_ccb[12];
unsigned char res, key, asc, ascq;
@@ -421,8 +420,8 @@ error:
 #define ATAPI_READ_BLOCK_SIZE  2048/* assuming CD part */
 #define ATAPI_READ_MAX_BLOCK   (ATAPI_READ_MAX_BYTES/ATAPI_READ_BLOCK_SIZE)
 
-ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-void *buffer)
+static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+   void *buffer)
 {
struct blk_desc *block_dev = dev_get_uclass_plat(dev);
int device = block_dev->devnum;
@@ -810,8 +809,8 @@ static void ide_input_data(int dev, ulong *sect_buf, int 
words)
}
 }
 
-ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-  void *buffer)
+static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+ void *buffer)
 {
struct blk_desc *block_dev = dev_get_uclass_plat(dev);
int device = block_dev->devnum;
@@ -930,8 +929,8 @@ IDE_READ_E:
return n;
 }
 
-ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-   const void *buffer)
+static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+  const void *buffer)
 {
struct blk_desc *block_dev = dev_get_uclass_plat(dev);
int device = block_dev->devnum;
diff --git a/include/ide.h b/include/ide.h
index b586ba3df4b..2c25e74ede0 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -11,17 +11,6 @@
 
 #define IDE_BUS(dev)   (dev / (CONFIG_SYS_IDE_MAXDEVICE / 
CONFIG_SYS_IDE_MAXBUS))
 
-/*
- * Function Prototypes
- */
-
-struct blk_desc;
-struct udevice;
-ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-  void *buffer);
-ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-   const void *buffer);
-
 /**
  * ide_set_reset() - Assert or de-assert reset for the IDE device
  *
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 12/30] ide: Change the retries variable

2023-04-25 Thread Simon Glass
Use a 'tries' variable which starts at the number of tries we want to do,
rather than a 'retries' one that stops at either 1 or 2. This will make it
easier to refactor the code to avoid the horrible #ifdefs

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 1d5e54d6eb9..782780fd302 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -557,7 +557,7 @@ static void ide_ident(struct blk_desc *dev_desc)
hd_driveid_t iop;
 #ifdef CONFIG_ATAPI
bool is_atapi = false;
-   int retries = 0;
+   int tries = 1;
 #endif
int device;
 
@@ -570,10 +570,10 @@ static void ide_ident(struct blk_desc *dev_desc)
dev_desc->uclass_id = UCLASS_IDE;
 #ifdef CONFIG_ATAPI
 
-   retries = 0;
+   tries = 2;
 
/* Warning: This will be tricky to read */
-   while (retries <= 1) {
+   while (tries) {
/* check signature */
if ((ide_inb(device, ATA_SECT_CNT) == 0x01) &&
(ide_inb(device, ATA_SECT_NUM) == 0x01) &&
@@ -624,7 +624,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 */
ide_outb(device, ATA_DEV_HD,
 ATA_LBA | ATA_DEVICE(device));
-   retries++;
+   tries--;
 #else
return;
 #endif
@@ -634,7 +634,7 @@ static void ide_ident(struct blk_desc *dev_desc)
break;
}   /* see above - ugly to read */
 
-   if (retries == 2)   /* Not found */
+   if (!tries) /* Not found */
return;
 #endif
 
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 10/30] ide: Correct use of ATAPI

2023-04-25 Thread Simon Glass
The use of atapi_read() was incorrect dropped. Fix this so that it will
be used when needed. Use a udevice for the first argument of atapi_read()
so it is consistent with ide_read().

This requires much of the ATAPI code to be brought out from behind the
existing #ifdef. It will still be removed by the compiler if it is not
needed.

Add an atapi flag to struct blk_desc so the information can be retained.

Fixes: 145df842b44 ("dm: ide: Add support for driver-model block devices")
Fixes: d0075059e4d ("ide: Drop non-DM code for BLK")
Reviewed-by: Mattijs Korpershoek 

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 20 +---
 include/blk.h   |  1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index fa5f68ffeb0..875192cba16 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -155,7 +155,6 @@ OUT:
*last = '\0';
 }
 
-#ifdef CONFIG_ATAPI
 /
  * ATAPI Support
  */
@@ -422,9 +421,10 @@ error:
 #define ATAPI_READ_BLOCK_SIZE  2048/* assuming CD part */
 #define ATAPI_READ_MAX_BLOCK   (ATAPI_READ_MAX_BYTES/ATAPI_READ_BLOCK_SIZE)
 
-ulong atapi_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
+ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 void *buffer)
 {
+   struct blk_desc *block_dev = dev_get_uclass_plat(dev);
int device = block_dev->devnum;
ulong n = 0;
unsigned char ccb[12];  /* Command descriptor block */
@@ -466,6 +466,8 @@ ulong atapi_read(struct blk_desc *block_dev, lbaint_t 
blknr, lbaint_t blkcnt,
return n;
 }
 
+#ifdef CONFIG_ATAPI
+
 static void atapi_inquiry(struct blk_desc *dev_desc)
 {
unsigned char ccb[12];  /* Command descriptor block */
@@ -653,6 +655,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 
 #ifdef CONFIG_ATAPI
if (is_atapi) {
+   dev_desc->atapi = true;
atapi_inquiry(dev_desc);
return;
}
@@ -1010,6 +1013,17 @@ WR_OUT:
return n;
 }
 
+ulong ide_or_atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+   void *buffer)
+{
+   struct blk_desc *desc = dev_get_uclass_plat(dev);
+
+   if (IS_ENABLED(CONFIG_ATAPI) && desc->atapi)
+   return atapi_read(dev, blknr, blkcnt, buffer);
+
+   return ide_read(dev, blknr, blkcnt, buffer);
+}
+
 static int ide_blk_probe(struct udevice *udev)
 {
struct blk_desc *desc = dev_get_uclass_plat(udev);
@@ -1029,7 +1043,7 @@ static int ide_blk_probe(struct udevice *udev)
 }
 
 static const struct blk_ops ide_blk_ops = {
-   .read   = ide_read,
+   .read   = ide_or_atapi_read,
.write  = ide_write,
 };
 
diff --git a/include/blk.h b/include/blk.h
index 1db203c1bab..871922dcde0 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -66,6 +66,7 @@ struct blk_desc {
/* device can use 48bit addr (ATA/ATAPI v7) */
unsigned char   lba48;
 #endif
+   unsigned char   atapi;  /* Use ATAPI protocol */
lbaint_tlba;/* number of blocks */
unsigned long   blksz;  /* block size */
int log2blksz;  /* for convenience: log2(blksz) */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 09/30] ide: Create a prototype for ide_set_reset()

2023-04-25 Thread Simon Glass
This is used by a board so should be in the header file. Add it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c |  2 --
 include/ide.h   | 10 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 46e110fec5e..fa5f68ffeb0 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -50,8 +50,6 @@ struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 #define IDE_SPIN_UP_TIME_OUT 5000 /* 5 sec spin-up timeout */
 
 #ifdef CONFIG_IDE_RESET
-extern void ide_set_reset(int idereset);
-
 static void ide_reset(void)
 {
int i;
diff --git a/include/ide.h b/include/ide.h
index 8c0eb2a022f..b586ba3df4b 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -22,4 +22,14 @@ ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t 
blkcnt,
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
const void *buffer);
 
+/**
+ * ide_set_reset() - Assert or de-assert reset for the IDE device
+ *
+ * This is provided by boards which need to reset the device through another
+ * means, e.g. a GPIO.
+ *
+ * @idereset: 1 to assert reset, 0 to de-assert it
+ */
+void ide_set_reset(int idereset);
+
 #endif /* _IDE_H */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 08/30] ide: Drop weak functions

2023-04-25 Thread Simon Glass
These are not used from outside this file anymore. Make them static and
remove them from the header file.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 14 +++---
 include/ide.h   | 13 -
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 58f1ef8f177..46e110fec5e 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -75,7 +75,7 @@ static void ide_reset(void)
 #define ide_reset()/* dummy */
 #endif /* CONFIG_IDE_RESET */
 
-__weak void ide_outb(int dev, int port, unsigned char val)
+static void ide_outb(int dev, int port, unsigned char val)
 {
debug("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
  dev, port, val, ATA_CURR_BASE(dev) + port);
@@ -83,7 +83,7 @@ __weak void ide_outb(int dev, int port, unsigned char val)
outb(val, ATA_CURR_BASE(dev) + port);
 }
 
-__weak unsigned char ide_inb(int dev, int port)
+static unsigned char ide_inb(int dev, int port)
 {
uchar val;
 
@@ -94,7 +94,7 @@ __weak unsigned char ide_inb(int dev, int port)
return val;
 }
 
-__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words)
+static void ide_input_swap_data(int dev, ulong *sect_buf, int words)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ushort *dbuf = (ushort *)sect_buf;
@@ -164,7 +164,7 @@ OUT:
 
 /* since ATAPI may use commands with not 4 bytes alligned length
  * we have our own transfer functions, 2 bytes alligned */
-__weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
+static void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ushort *dbuf;
@@ -179,7 +179,7 @@ __weak void ide_output_data_shorts(int dev, ushort 
*sect_buf, int shorts)
}
 }
 
-__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
+static void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ushort *dbuf;
@@ -778,7 +778,7 @@ static void ide_init(void)
schedule();
 }
 
-__weak void ide_output_data(int dev, const ulong *sect_buf, int words)
+static void ide_output_data(int dev, const ulong *sect_buf, int words)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ushort *dbuf;
@@ -792,7 +792,7 @@ __weak void ide_output_data(int dev, const ulong *sect_buf, 
int words)
}
 }
 
-__weak void ide_input_data(int dev, ulong *sect_buf, int words)
+static void ide_input_data(int dev, ulong *sect_buf, int words)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ushort *dbuf;
diff --git a/include/ide.h b/include/ide.h
index 09b0117879f..8c0eb2a022f 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -22,17 +22,4 @@ ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t 
blkcnt,
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
const void *buffer);
 
-/*
- * I/O function overrides
- */
-unsigned char ide_inb(int dev, int port);
-void ide_outb(int dev, int port, unsigned char val);
-void ide_input_swap_data(int dev, ulong *sect_buf, int words);
-void ide_input_data(int dev, ulong *sect_buf, int words);
-void ide_output_data(int dev, const ulong *sect_buf, int words);
-void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts);
-void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts);
-
-void ide_led(uchar led, uchar status);
-
 #endif /* _IDE_H */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 07/30] ide: Move a few functions further up the file

2023-04-25 Thread Simon Glass
Move these functions so they appear before they are used.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 68 ++---
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index b5be022a067..58f1ef8f177 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -75,6 +75,40 @@ static void ide_reset(void)
 #define ide_reset()/* dummy */
 #endif /* CONFIG_IDE_RESET */
 
+__weak void ide_outb(int dev, int port, unsigned char val)
+{
+   debug("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
+ dev, port, val, ATA_CURR_BASE(dev) + port);
+
+   outb(val, ATA_CURR_BASE(dev) + port);
+}
+
+__weak unsigned char ide_inb(int dev, int port)
+{
+   uchar val;
+
+   val = inb(ATA_CURR_BASE(dev) + port);
+
+   debug("ide_inb (dev= %d, port= 0x%x) : @ 0x%08lx -> 0x%02x\n",
+ dev, port, ATA_CURR_BASE(dev) + port, val);
+   return val;
+}
+
+__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words)
+{
+   uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
+   ushort *dbuf = (ushort *)sect_buf;
+
+   debug("in input swap data base for read is %p\n", (void *)paddr);
+
+   while (words--) {
+   EIEIO;
+   *dbuf++ = be16_to_cpu(inw(paddr));
+   EIEIO;
+   *dbuf++ = be16_to_cpu(inw(paddr));
+   }
+}
+
 /*
  * Wait until Busy bit is off, or timeout (in ms)
  * Return last status
@@ -668,25 +702,6 @@ static void ide_ident(struct blk_desc *dev_desc)
 #endif
 }
 
-__weak void ide_outb(int dev, int port, unsigned char val)
-{
-   debug("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
- dev, port, val, ATA_CURR_BASE(dev) + port);
-
-   outb(val, ATA_CURR_BASE(dev) + port);
-}
-
-__weak unsigned char ide_inb(int dev, int port)
-{
-   uchar val;
-
-   val = inb(ATA_CURR_BASE(dev) + port);
-
-   debug("ide_inb (dev= %d, port= 0x%x) : @ 0x%08lx -> 0x%02x\n",
- dev, port, ATA_CURR_BASE(dev) + port, val);
-   return val;
-}
-
 static void ide_init(void)
 {
unsigned char c;
@@ -763,21 +778,6 @@ static void ide_init(void)
schedule();
 }
 
-__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words)
-{
-   uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
-   ushort *dbuf = (ushort *)sect_buf;
-
-   debug("in input swap data base for read is %p\n", (void *)paddr);
-
-   while (words--) {
-   EIEIO;
-   *dbuf++ = be16_to_cpu(inw(paddr));
-   EIEIO;
-   *dbuf++ = be16_to_cpu(inw(paddr));
-   }
-}
-
 __weak void ide_output_data(int dev, const ulong *sect_buf, int words)
 {
uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 05/30] ide: Move ide_init() into probing

2023-04-25 Thread Simon Glass
At present the code does ide_init() as a separate operation, then calls
device_probe() to copy over the information. We can call ide_init() from
probe just as easily.

The only difference is that using 'ide init' twice will do nothing.
However it already fails to copy over the new data in that case, so the
effect is the same. For now, unbind the block devices and remove the IDE
device, which causes the bus to be probed again. Later patches will fix
this up fully, so that all blk_desc data is copied across.

Since ide_reset() is only called from ide_init(), there is no need to init
the ide_dev_desc[] array. This is already done at the end of ide_init() so
drop this code.

The call to uclass_first_device() is now within the probe() function of
the same device, so does nothing. Drop it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 cmd/ide.c   | 22 +-
 drivers/block/ide.c | 13 ++---
 include/ide.h   |  1 -
 test/boot/bootdev.c |  2 --
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/cmd/ide.c b/cmd/ide.c
index 6739f0b12d1..ddc87d3a0bb 100644
--- a/cmd/ide.c
+++ b/cmd/ide.c
@@ -10,12 +10,15 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -31,8 +34,25 @@ int do_ide(struct cmd_tbl *cmdtp, int flag, int argc, char 
*const argv[])
 {
if (argc == 2) {
if (strncmp(argv[1], "res", 3) == 0) {
+   struct udevice *dev;
+   int ret;
+
puts("\nReset IDE: ");
-   ide_init();
+   ret = uclass_find_first_device(UCLASS_IDE, );
+   ret = device_remove(dev, DM_REMOVE_NORMAL);
+   if (!ret)
+   ret = device_chld_unbind(dev, NULL);
+   if (ret) {
+   printf("Cannot remove IDE (err=%dE)\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   ret = uclass_first_device_err(UCLASS_IDE, );
+   if (ret) {
+   printf("Init failed (err=%dE)\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
return 0;
}
}
diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 6f601bcf864..13770484b36 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -58,8 +58,6 @@ static void ide_reset(void)
 
for (i = 0; i < CONFIG_SYS_IDE_MAXBUS; ++i)
ide_bus_ok[i] = 0;
-   for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i)
-   ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
 
ide_set_reset(1);   /* assert reset */
 
@@ -689,9 +687,8 @@ __weak unsigned char ide_inb(int dev, int port)
return val;
 }
 
-void ide_init(void)
+static void ide_init(void)
 {
-   struct udevice *dev;
unsigned char c;
int i, bus;
 
@@ -764,8 +761,6 @@ void ide_init(void)
dev_print(_dev_desc[i]);
}
schedule();
-
-   uclass_first_device(UCLASS_IDE, );
 }
 
 __weak void ide_input_swap_data(int dev, ulong *sect_buf, int words)
@@ -1067,7 +1062,9 @@ static int ide_bootdev_bind(struct udevice *dev)
 
 static int ide_bootdev_hunt(struct bootdev_hunter *info, bool show)
 {
-   ide_init();
+   struct udevice *dev;
+
+   uclass_first_device(UCLASS_IDE, );
 
return 0;
 }
@@ -1104,6 +1101,8 @@ static int ide_probe(struct udevice *udev)
int i;
int ret;
 
+   ide_init();
+
for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
sprintf(name, "blk#%d", i);
diff --git a/include/ide.h b/include/ide.h
index 457f275c61b..3f36b4340d0 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -15,7 +15,6 @@
  * Function Prototypes
  */
 
-void ide_init(void);
 struct blk_desc;
 struct udevice;
 ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 4fe9fd72208..365b7f518ab 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -376,7 +376,6 @@ static int bootdev_test_cmd_hunt(struct unit_test_state 
*uts)
ut_assert_nextline("Hunting with: simple_bus");
ut_assert_nextline("Found 2 extension board(s).");
ut_assert_nextline("Hunting with: ide");
-   ut_assert_nextline("Bus 0: not available  ");
 
/* mmc hunter has already been used so should not run again */
 
@@ -487,7 +486,6 @@ static int bootdev_test_hunt_prio(struct unit_test_state 
*uts)
/* now try a different priority, verbosely */
ut_assertok(bootdev_hunt_prio(BOOTDEVP_5_SCAN_SLOW, true));
ut_assert_nextline("Hunting with: ide");
-   ut_assert_nextline("Bus 0: not 

[PATCH v2 06/30] ide: Drop ide_device_present()

2023-04-25 Thread Simon Glass
This function is not used anymore. Drop it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 9 -
 include/ide.h   | 4 
 2 files changed, 13 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 13770484b36..b5be022a067 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1012,15 +1012,6 @@ WR_OUT:
return n;
 }
 
-#if defined(CONFIG_OF_IDE_FIXUP)
-int ide_device_present(int dev)
-{
-   if (dev >= CONFIG_SYS_IDE_MAXBUS)
-   return 0;
-   return ide_dev_desc[dev].type == DEV_TYPE_UNKNOWN ? 0 : 1;
-}
-#endif
-
 static int ide_blk_probe(struct udevice *udev)
 {
struct blk_desc *desc = dev_get_uclass_plat(udev);
diff --git a/include/ide.h b/include/ide.h
index 3f36b4340d0..09b0117879f 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -22,10 +22,6 @@ ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t 
blkcnt,
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
const void *buffer);
 
-#if defined(CONFIG_OF_IDE_FIXUP)
-int ide_device_present(int dev);
-#endif
-
 /*
  * I/O function overrides
  */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 04/30] ide: Drop init for not using BLK

2023-04-25 Thread Simon Glass
ALl boards use CONFIG_BLK now so this code is not used. Drop it and the
header-file #ifdef

Signed-off-by: Simon Glass 
---

(no changes since v1)

 common/board_r.c | 12 
 include/ide.h|  7 ---
 2 files changed, 19 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 7076af64f5d..d798c00a80a 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -519,15 +519,6 @@ static int initr_post(void)
 }
 #endif
 
-#if defined(CONFIG_IDE) && !defined(CONFIG_BLK)
-static int initr_ide(void)
-{
-   puts("IDE:   ");
-   ide_init();
-   return 0;
-}
-#endif
-
 #if defined(CFG_PRAM)
 /*
  * Export available size of memory for Linux, taking into account the
@@ -778,9 +769,6 @@ static init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_POST
initr_post,
 #endif
-#if defined(CONFIG_IDE) && !defined(CONFIG_BLK)
-   initr_ide,
-#endif
 #ifdef CONFIG_LAST_STAGE_INIT
INIT_FUNC_WATCHDOG_RESET
/*
diff --git a/include/ide.h b/include/ide.h
index 9c0d40364a8..457f275c61b 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -18,17 +18,10 @@
 void ide_init(void);
 struct blk_desc;
 struct udevice;
-#ifdef CONFIG_BLK
 ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
   void *buffer);
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
const void *buffer);
-#else
-ulong ide_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
-  void *buffer);
-ulong ide_write(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
-   const void *buffer);
-#endif
 
 #if defined(CONFIG_OF_IDE_FIXUP)
 int ide_device_present(int dev);
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 03/30] ide: Drop CONFIG_START_IDE

2023-04-25 Thread Simon Glass
This is not used by any board. Drop it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 common/board_r.c | 5 -
 include/ide.h| 7 ---
 2 files changed, 12 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 6b4180b3ecd..7076af64f5d 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -523,12 +523,7 @@ static int initr_post(void)
 static int initr_ide(void)
 {
puts("IDE:   ");
-#if defined(CONFIG_START_IDE)
-   if (board_start_ide())
-   ide_init();
-#else
ide_init();
-#endif
return 0;
 }
 #endif
diff --git a/include/ide.h b/include/ide.h
index 58f6640c61b..9c0d40364a8 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -47,11 +47,4 @@ void ide_output_data_shorts(int dev, ushort *sect_buf, int 
shorts);
 
 void ide_led(uchar led, uchar status);
 
-/**
- * board_start_ide() - Start up the board IDE interfac
- *
- * Return: 0 if ok
- */
-int board_start_ide(void);
-
 #endif /* _IDE_H */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 02/30] ide: Use mdelay() for long delays

2023-04-25 Thread Simon Glass
Rather than using very large numbers with udelay(), use mdelay(), which
is easier to follow.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index f36bec8b3a8..6f601bcf864 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -71,9 +71,7 @@ static void ide_reset(void)
/* de-assert RESET signal */
ide_set_reset(0);
 
-   /* wait 250 ms */
-   for (i = 0; i < 250; ++i)
-   udelay(1000);
+   mdelay(250);
 }
 #else
 #define ide_reset()/* dummy */
@@ -237,7 +235,7 @@ unsigned char atapi_issue(int device, unsigned char *ccb, 
int ccblen,
ide_output_data_shorts(device, (unsigned short *)ccb, ccblen / 2);
 
/* ATAPI Command written wait for completition */
-   udelay(5000);   /* device must set bsy */
+   mdelay(5);  /* device must set bsy */
 
mask = ATA_STAT_DRQ | ATA_STAT_BUSY | ATA_STAT_ERR;
/*
@@ -293,7 +291,7 @@ unsigned char atapi_issue(int device, unsigned char *ccb, 
int ccblen,
  n);
}
}
-   udelay(5000);   /* seems that some CD ROMs need this... */
+   mdelay(5);  /* seems that some CD ROMs need this... */
mask = ATA_STAT_BUSY | ATA_STAT_ERR;
res = 0;
c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
@@ -357,7 +355,7 @@ retry:
 
if ((key == 6) || (asc == 0x29) || (asc == 0x28)) { /* Unit Attention */
if (unitattn-- > 0) {
-   udelay(200 * 1000);
+   mdelay(200);
goto retry;
}
printf("Unit Attention, tried %d\n", ATAPI_UNIT_ATTN);
@@ -366,7 +364,7 @@ retry:
if ((asc == 0x4) && (ascq == 0x1)) {
/* not ready, but will be ready soon */
if (notready-- > 0) {
-   udelay(200 * 1000);
+   mdelay(200);
goto retry;
}
printf("Drive not ready, tried %d times\n",
@@ -586,9 +584,9 @@ static void ide_ident(struct blk_desc *dev_desc)
debug("Retrying...\n");
ide_outb(device, ATA_DEV_HD,
 ATA_LBA | ATA_DEVICE(device));
-   udelay(10);
+   mdelay(100);
ide_outb(device, ATA_COMMAND, 0x08);
-   udelay(50); /* 500 ms */
+   mdelay(500);
}
/*
 * Select device
@@ -715,14 +713,13 @@ void ide_init(void)
 
ide_bus_ok[bus] = 0;
 
-   /* Select device
-*/
-   udelay(10); /* 100 ms */
+   /* Select device */
+   mdelay(100);
ide_outb(dev, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(dev));
-   udelay(10); /* 100 ms */
+   mdelay(100);
i = 0;
do {
-   udelay(1);  /* 10 ms */
+   mdelay(10);
 
c = ide_inb(dev, ATA_STATUS);
i++;
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 01/30] ide: Move ATA_CURR_BASE to C file

2023-04-25 Thread Simon Glass
This is not used outside one C file. Move it out of the header to
reduce its visbility.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 drivers/block/ide.c | 3 +++
 include/ide.h   | 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 1ad9b6c1267..f36bec8b3a8 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -36,6 +36,9 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 #endif
 };
 
+#define ATA_CURR_BASE(dev) (CONFIG_SYS_ATA_BASE_ADDR + \
+   ide_bus_offset[IDE_BUS(dev)])
+
 static int ide_bus_ok[CONFIG_SYS_IDE_MAXBUS];
 
 struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
diff --git a/include/ide.h b/include/ide.h
index 426cef4e39e..58f6640c61b 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -11,9 +11,6 @@
 
 #define IDE_BUS(dev)   (dev / (CONFIG_SYS_IDE_MAXDEVICE / 
CONFIG_SYS_IDE_MAXBUS))
 
-#defineATA_CURR_BASE(dev)  
(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])
-extern ulong ide_bus_offset[];
-
 /*
  * Function Prototypes
  */
-- 
2.40.0.634.g4ca3ef3211-goog



[PATCH v2 00/30] ide: Clean up code and fix a few bugs

2023-04-25 Thread Simon Glass
This code was converted to driver model a long time again but it was a
pretty rough conversion. It introduced a few minor bugs, e.g. the device
capacity is incorrect and some flags are lost (such as lba48).

This series tidies up the code and fixes these bugs. This involves quite
a bit of refactoring, so it is done one patch at a time for easier
review.

Changes in v2:
- Drop [] in patch subject
- Rebase to master

Simon Glass (30):
  ide: Move ATA_CURR_BASE to C file
  ide: Use mdelay() for long delays
  ide: Drop CONFIG_START_IDE
  ide: Drop init for not using BLK
  ide: Move ide_init() into probing
  ide: Drop ide_device_present()
  ide: Move a few functions further up the file
  ide: Drop weak functions
  ide: Create a prototype for ide_set_reset()
  ide: Correct use of ATAPI
  ide: Make function static
  ide: Change the retries variable
  ide: Refactor confusing loop code
  ide: Simplify success condition
  ide: Avoid preprocessor for CONFIG_ATAPI
  ide: Avoid preprocessor for CONFIG_LBA48
  ide: Move bus init into a function
  ide: Make ide_bus_ok a local variable
  ide: Move setting of vendor strings into ide_probe()
  ide: Move ide_init() entirely within ide_probe()
  ide: Combine the two loops in ide_probe()
  ide: Use desc consistently for struct blk_desc
  ide: Make ide_ident() return an error code
  ide: Move all blk_desc init into ide_ident()
  ide: Use a single local blk_desc for ide_ident()
  ide: Correct LBA setting
  ide: Tidy up ide_reset()
  ide: Convert to use log_debug()
  ide: Simplify expressions and hex values
  ide: Make use of U-Boot types

 cmd/ide.c   |  22 +-
 common/board_r.c|  17 -
 drivers/block/ide.c | 841 
 include/blk.h   |   5 +-
 include/ide.h   |  48 +--
 test/boot/bootdev.c |   2 -
 6 files changed, 415 insertions(+), 520 deletions(-)

-- 
2.40.0.634.g4ca3ef3211-goog



Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

2023-04-25 Thread Pali Rohár
On Tuesday 25 April 2023 10:44:17 Bin Meng wrote:
> Hi Simon,
> 
> On Tue, Apr 25, 2023 at 7:09 AM Simon Glass  wrote:
> >
> > The sandbox build makes use of a small number of weak symbols. Allow these
> > to be dropped when building for the PE format, since its support for weak
> > symbols is poor.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  cmd/bootefi.c | 3 ++-
> >  cmd/bootz.c   | 3 +++
> >  common/usb.c  | 3 +++
> >  drivers/core/root.c   | 3 +++
> >  drivers/spi/sandbox_spi.c | 3 +++
> >  env/env.c | 6 ++
> >  lib/efi_loader/efi_image_loader.c | 3 +++
> >  lib/efi_loader/efi_runtime.c  | 4 
> >  lib/lmb.c | 4 +++-
> >  lib/time.c| 3 +++
> >  10 files changed, 33 insertions(+), 2 deletions(-)
> >
> 
> You probably need to use:
> 
> __declspec(selectany)
> 
> to replace __weak in the ELF for the *nix world.
> 
> Note this Microsoft bizarre does not provide the "strong override
> weak" effect but I suspect what you only need is to get the build pass
> on Windows so it should be okay.
> 
> Regards,
> Bin

You have selectany and external weaks support in PE format. For the
majority of weak function cases, this is enough. Cannot we use them?


Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

2023-04-25 Thread Pali Rohár
On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> This creates a lot of errors of the form:
> 
> `__stack_chk_fail' referenced in section `.text' of ...ltrans.o: defined
>in discarded section `.text' of common/stackprot.o (symbol from plugin)

This issue should be rather fixed...

> Drop LTO for now.

... and until it happens is not CONFIG_LTO for disabling enough?

LTO does not work for more other boards / platforms and it is just _not_
enabled via CONFIG_LTO in those cases...

> Signed-off-by: Simon Glass 
> ---
> 
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 08eab87d0a11..3a35d14a4497 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -658,9 +658,11 @@ export EFI_TARGET# binutils target if EFI is 
> natively supported
>  export LTO_ENABLE
>  
>  # This is y if LTO is enabled for this build. See NO_LTO=1 to disable LTO
> +ifeq ($(MSYS_VERSION),0)
>  ifeq ($(NO_LTO),)
>  LTO_ENABLE=$(if $(CONFIG_LTO),y)
>  endif
> +endif
>  
>  # If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use
>  # that (or fail if absent).  Otherwise, search for a linker script in a
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 


[PATCH] fdt: Indicate that people should use the ofnode API

2023-04-25 Thread Simon Glass
Add a note to the comment at the top of this file.

Signed-off-by: Simon Glass 
---

 lib/fdtdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 0827e16859f..55cc95db5d0 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
  * Copyright (c) 2011 The Chromium OS Authors.
+ *
+ * NOTE: Please do not add new devicetree-reading functionality into this file.
+ * Add it to the ofnode API instead, since that is compatible with livetree.
  */
 
 #ifndef USE_HOSTCC
-- 
2.40.0.634.g4ca3ef3211-goog



Re: [PATCH 26/31] Makefile: Correct the ans1_compiler rule for MSYS2

2023-04-25 Thread Pali Rohár
On Monday 24 April 2023 17:08:31 Simon Glass wrote:
> Add the required extension to the Makefile rule.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  Makefile   | 1 +
>  scripts/Makefile.build | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 773260b81d5d..08eab87d0a11 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -48,6 +48,7 @@ ifeq ($(MSYS_VERSION),0)
>  export SOEXT := so
>  else
>  export SOEXT := dll
> +export ELFEXT := .exe

.EXE is not extension for ELF format :-)

Autotools and also other tools use EXEEXT Makefile variable for this.
What about following this de-factor standard variable name?

>  endif
>  
>  # Avoid funny character set dependencies
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 97dd4a64f6ef..547dc039ceb4 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -309,7 +309,7 @@ quiet_cmd_asn1_compiler = ASN.1   $@
>cmd_asn1_compiler = $(objtree)/tools/asn1_compiler $< \
>   $(subst .h,.c,$@) $(subst .c,.h,$@)
>  
> -$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler
> +$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 
> $(objtree)/tools/asn1_compiler$(ELFEXT)
>   $(call cmd,asn1_compiler)
>  
>  # Build the compiled-in targets
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 


Re: [PATCH 25/31] Makefile: Disable unsupported compiler options with PE

2023-04-25 Thread Pali Rohár
On Monday 24 April 2023 17:08:30 Simon Glass wrote:
> The MSYS2 compiler does not support some of these options. Drop them to
> avoid build errors.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  Makefile | 2 ++
>  scripts/Makefile.lib | 5 -
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 0aa97a2c3b48..773260b81d5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -818,7 +818,9 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS)
>  KBUILD_AFLAGS += $(KAFLAGS)
>  KBUILD_CFLAGS += $(KCFLAGS)
>  
> +ifeq ($(MSYS_VERSION),0)
>  KBUILD_LDFLAGS  += -z noexecstack

What about rather using $(call ...) instead of ifeq?

> +endif
>  KBUILD_LDFLAGS  += $(call ld-option,--no-warn-rwx-segments)
>  
>  KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g)
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 7b27224b5d44..d309e26aaec8 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -425,7 +425,10 @@ cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j 
> .sdata -j .data -j \
>  $(obj)/%.efi: $(obj)/%_efi.so
>   $(call cmd,efi_objcopy)
>  
> -KBUILD_EFILDFLAGS = -nostdlib -zexecstack -znocombreloc -znorelro
> +KBUILD_EFILDFLAGS = -nostdlib
> +ifeq ($(MSYS_VERSION),0)
> +KBUILD_EFILDFLAGS += -zexecstack -znocombreloc -znorelro
> +endif
>  KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments)
>  quiet_cmd_efi_ld = LD  $@
>  cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 


Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

2023-04-25 Thread Pali Rohár
On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> Add a script to allow the U-Boot sandbox executable to be built for
> Windows. Add a note as to why this seems to be necessary for now.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  Makefile   |  11 +-
>  arch/sandbox/config.mk |   4 +
>  arch/sandbox/cpu/u-boot-pe.lds | 447 +
>  3 files changed, 460 insertions(+), 2 deletions(-)
>  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> 
> diff --git a/Makefile b/Makefile
> index dd3fcd1782e5..0aa97a2c3b48 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1730,6 +1730,13 @@ else
>  u-boot-keep-syms-lto :=
>  endif
>  
> +ifeq ($(MSYS_VERSION),0)
> +add_ld_script := -T u-boot.lds
> +else
> +add_ld_script := u-boot.lds
> +$(warning msys)
> +endif
> +
>  # Rule to link u-boot
>  # May be overridden by arch/$(ARCH)/config.mk
>  ifeq ($(LTO_ENABLE),y)
> @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO $@
>   $(CC) -nostdlib -nostartfiles   
> \
>   $(LTO_FINAL_LDFLAGS) $(c_flags) 
> \
>   $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@   
> \
> - -T u-boot.lds $(u-boot-init)
> \
> + $(add_ld_script) $(u-boot-init) 
> \
>   -Wl,--whole-archive 
> \
>   $(u-boot-main)  
> \
>   $(u-boot-keep-syms-lto) 
> \
> @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO $@
>  else
>  quiet_cmd_u-boot__ ?= LD  $@
>cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@
> \
> - -T u-boot.lds $(u-boot-init)
> \
> + $(add_ld_script) $(u-boot-init) 
> \
>   --whole-archive 
> \
>   $(u-boot-main)  
> \
>   --no-whole-archive  
> \
> diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> index 2d184c5f652a..e05daf57ef8e 100644
> --- a/arch/sandbox/config.mk
> +++ b/arch/sandbox/config.mk
> @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
>  EFI_RELOC := reloc_sandbox_efi.o
>  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
>  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> +
> +ifneq ($(MSYS_VERSION),0)
> +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> +endif
> diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> new file mode 100644
> index ..031e70fafd03
> --- /dev/null
> +++ b/arch/sandbox/cpu/u-boot-pe.lds
> @@ -0,0 +1,447 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * U-Boot note: This was obtained by using the -verbose linker option. The
> + * U-Boot additions are marked below.
> + *
> + * Ideally we would add sections to the executable, as is done with the Linux
> + * build. But PE executables do not appear to work correctly if unexpected
> + * sections are present:
> + *
> + *   $ /tmp/b/sandbox/u-boot.exe
> + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec 
> format error
> + *
> + * So we take a approach of rewriting the whole file, for now. This will 
> likely
> + * break in the future when a toolchain change is made.

Why not rather provide "layer" linker script which does this "rewriting"
on top of the default linker script? With this way it is not needed to
update linker script when a toolchain change it.

> + */
> +
> +/* Default linker script, for normal executables */
> +/* Copyright (C) 2014-2023 Free Software Foundation, Inc.
> +   Copying and distribution of this script, with or without modification,
> +   are permitted in any medium without royalty provided the copyright
> +   notice and this notice are preserved.  */
> +
> +OUTPUT_FORMAT(pei-x86-64)
> +SEARCH_DIR("/usr/x86_64-pc-msys/lib"); SEARCH_DIR("/usr/lib"); 
> SEARCH_DIR("/usr/lib/w32api");
> +SECTIONS
> +{
> +  /* Make the virtual address and file offset synced if the alignment is
> + lower than the target page size. */
> +  . = SIZEOF_HEADERS;
> +  . = ALIGN(__section_alignment__);
> +  .text  __image_base__ + ( __section_alignment__ < 0x1000 ? . : 
> __section_alignment__ ) :
> +  {
> +KEEP (*(SORT_NONE(.init)))
> +*(.text)
> +*(SORT(.text$*))
> + *(.text.*)
> + *(.gnu.linkonce.t.*)
> +*(.glue_7t)
> +*(.glue_7)
> +. = ALIGN(8);
> +   /* Note: we always define __CTOR_LIST__ and ___CTOR_LIST__ here,
> +  we do not PROVIDE them.  This is because the ctors.o startup
> +   code in libgcc defines them as common symbols, with the
> +  expectation that they will be 

Re: [PATCH 24/31] build: Disable weak symbols for MSYS2

2023-04-25 Thread Pali Rohár
On Monday 24 April 2023 17:08:29 Simon Glass wrote:
> Weak symbols are not well supported by the PE format, so disable them.

They are supported by PE format. This is likely issue of (older)
toolchain. What about rather requiring better toolchain version and
fix special cases of weak functions do not work correctly?

> We need to manually ensure that only one function is present in the source
> code.
> 
> Add a Kconfig option to control this and enable it when building for
> Windows.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  Kconfig | 15 +++
>  include/linux/compiler_attributes.h |  4 
>  2 files changed, 19 insertions(+)
> 
> diff --git a/Kconfig b/Kconfig
> index f24e4f0a331e..ca1402d09d10 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -72,6 +72,21 @@ config CLANG_VERSION
>   int
>   default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
>  
> +config CC_IS_MSYS
> + def_bool $(success,uname -o | grep -q Msys)
> +
> +config WEAK_SYMBOLS
> + bool "Enable use of weak symbols"
> + default y if !CC_IS_MSYS
> + help
> +   The Portable Executable (PE) format used by Windows does not support
> +   weak symbols very well. Even where it can be made to work, the __weak
> +   function attribute cannot be made to work with PE. Supporting weak
> +   symbols would involve changing the source code in undesirable ways.
> +
> +   This option controls whether weak symbols are used, or not. When
> +   disabled, the __weak function attribute does nothing.
> +
>  choice
>   prompt "Optimization level"
>   default CC_OPTIMIZE_FOR_SIZE
> diff --git a/include/linux/compiler_attributes.h 
> b/include/linux/compiler_attributes.h
> index 44c9a08d7346..c954109a065b 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -268,6 +268,10 @@
>   *   gcc: 
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-weak-function-attribute
>   *   gcc: 
> https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
>   */
> +#ifdef CONFIG_WEAK_SYMBOLS
>  #define __weak  __attribute__((__weak__))
> +#else
> +#define __weak
> +#endif
>  
>  #endif /* __LINUX_COMPILER_ATTRIBUTES_H */
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 


[PATCH] environment: ti: Add get_fit_config command to get FIT config string

2023-04-25 Thread Andrew Davis
When OE is packaging a dtb file into the FIT image it names the node based
on the dtb filename. Node names can't have "/" so it is turned into "_".
We select our FIT config using the "fdtfile" env var so we don't duplicate
the board_name to fdt logic. Result is fdtfile needs mangled when used to
select a config node from OE made FIT image. Do this here.

Signed-off-by: Andrew Davis 
---
 include/configs/ti_armv7_common.h  | 3 ++-
 include/environment/ti/ti_armv7_common.env | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/configs/ti_armv7_common.h 
b/include/configs/ti_armv7_common.h
index d54c208ef66..149a74d98e8 100644
--- a/include/configs/ti_armv7_common.h
+++ b/include/configs/ti_armv7_common.h
@@ -55,7 +55,8 @@
"do;" \
"setenv overlaystring ${overlaystring}'#'${overlay};" \
"done;\0" \
-   "run_fit=bootm ${addr_fit}#conf-${fdtfile}${overlaystring}\0" \
+   "get_fit_config=setexpr name_fit_config gsub / _ conf-${fdtfile}\0" \
+   "run_fit=run get_fit_config; bootm 
${addr_fit}#${name_fit_config}${overlaystring}\0" \
 
 /*
  * DDR information.  If the CONFIG_NR_DRAM_BANKS is not defined,
diff --git a/include/environment/ti/ti_armv7_common.env 
b/include/environment/ti/ti_armv7_common.env
index 4d334648c05..0c0929d8628 100644
--- a/include/environment/ti/ti_armv7_common.env
+++ b/include/environment/ti/ti_armv7_common.env
@@ -20,5 +20,6 @@ get_overlaystring=
do;
setenv overlaystring ${overlaystring}'#'${overlay};
done;
-run_fit=bootm ${addr_fit}#conf-${fdtfile}${overlaystring}
+get_fit_config=setexpr name_fit_config gsub / _ conf-${fdtfile}
+run_fit=run get_fit_config; bootm 
${addr_fit}#${name_fit_config}${overlaystring}
 
-- 
2.39.2



Re: [PATCH 06/31] pylibfdt: Allow building on Windows

2023-04-25 Thread Pali Rohár
On Monday 24 April 2023 17:08:11 Simon Glass wrote:
> Handle the different shared-object extension with MSYS2 by creating a new
> SOEXT variable.

LIBEXT would be a better name as "SO" name refers to ELF.
I saw that variable LIBEXT is used in more projects for this purpose.

> Signed-off-by: Simon Glass 
> ---
> 
>  Makefile  | 11 +++
>  scripts/dtc/pylibfdt/Makefile | 16 +---
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index eaaf7d267d31..dd3fcd1782e5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -39,6 +39,17 @@ else ifeq ("riscv64", $(MK_ARCH))
>  endif
>  undefine MK_ARCH
>  
> +# Building on Windows with MSYS2
> +export MSYS_VERSION = $(if $(findstring Msys, $(shell uname -o)),$(word 1, 
> $(subst ., ,$(shell uname -r))),0)
> +# $(info The version of MSYS you are running is $(MSYS_VERSION) (0 meaning 
> not MSYS at all))
> +
> +# Sets the extension to use for shared-object files
> +ifeq ($(MSYS_VERSION),0)
> +export SOEXT := so
> +else
> +export SOEXT := dll
> +endif
> +
>  # Avoid funny character set dependencies
>  unexport LC_ALL
>  LC_COLLATE=C
> diff --git a/scripts/dtc/pylibfdt/Makefile b/scripts/dtc/pylibfdt/Makefile
> index e442d5c24201..feca0bb9abb1 100644
> --- a/scripts/dtc/pylibfdt/Makefile
> +++ b/scripts/dtc/pylibfdt/Makefile
> @@ -7,6 +7,8 @@ LIBFDT_srcdir = $(abspath $(srctree)/$(src)/../libfdt)
>  
>  include $(LIBFDT_srcdir)/Makefile.libfdt
>  
> +LIBFILE := _libfdt.$(SOEXT)
> +
>  # Unfortunately setup.py (or actually the Python distutil implementation) 
> puts
>  # files into the same directory as the .i file. We cannot touch the source
>  # directory, so we "ship" .i file into the objtree.
> @@ -29,16 +31,16 @@ quiet_cmd_pymod = PYMOD   $@
>  rebuild: $(src)/setup.py $(PYLIBFDT_srcs)
>   @# Remove the library since otherwise Python doesn't seem to regenerate
>   @# the libfdt.py file if it is missing.
> - @rm -f $(obj)/_libfdt*.so
> + @rm -f $(obj)/_libfdt*.$(SOEXT)
>   $(call if_changed,pymod)
> - @# Rename the file to _libfdt.so so this Makefile doesn't run every time
> - @if [ ! -e $(obj)/_libfdt.so ]; then \
> - mv $(obj)/_libfdt*.so $(obj)/_libfdt.so; \
> + @# Rename the file to $(LIBFILE) so this Makefile doesn't run every time
> + @if [ ! -e $(obj)/$(LIBFILE) ]; then \
> + mv $(obj)/_libfdt*.$(SOEXT) $(obj)/$(LIBFILE); \
>   fi
>  
> -$(obj)/_libfdt.so $(obj)/libfdt.py &: rebuild
> +$(obj)/$(LIBFILE) $(obj)/libfdt.py &: rebuild
>   @:
>  
> -always += _libfdt.so libfdt.py
> +always += $(LIBFILE) libfdt.py
>  
> -clean-files += libfdt.i _libfdt.so libfdt.py libfdt_wrap.c
> +clean-files += libfdt.i $(LIBFILE) libfdt.py libfdt_wrap.c
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 


Re: [PATCH 00/31] Allow building sandbox with MSYS2

2023-04-25 Thread Tom Rini
On Tue, Apr 25, 2023 at 04:59:53AM +0200, Heinrich Schuchardt wrote:
> 
> 
> Am 25. April 2023 01:08:05 MESZ schrieb Simon Glass :
> >This expands the existing work to allow sandbox to build and run on
> >Windows using MSYS2.
> 
> Why do we need this?
> Wouldn't a developer on Windows be much better served using WSL 
> (https://learn.microsoft.com/en-us/windows/wsl/install)?

Depending on corporate rules that may or may not be allowed. But I too
am interested in the use case driving these changes. Sandbox is
essentially a test platform. We test that we can do host tools on
Windows via MSYS2 for semi-reasonable reasons, since those tools can be
used in a number of places. We do the same on macOS in Azure as a check
on building regular targets there too (which should work and I believe
people do, both from macOS and from *BSD which this is a stand-in for,
as we can't get free VMs for them today). What's the end goal of this
series?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 25/31] Makefile: Disable unsupported compiler options with PE

2023-04-25 Thread Tom Rini
On Mon, Apr 24, 2023 at 05:08:30PM -0600, Simon Glass wrote:
> The MSYS2 compiler does not support some of these options. Drop them to
> avoid build errors.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  Makefile | 2 ++
>  scripts/Makefile.lib | 5 -
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 0aa97a2c3b48..773260b81d5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -818,7 +818,9 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS)
>  KBUILD_AFLAGS += $(KAFLAGS)
>  KBUILD_CFLAGS += $(KCFLAGS)
>  
> +ifeq ($(MSYS_VERSION),0)
>  KBUILD_LDFLAGS  += -z noexecstack
> +endif
>  KBUILD_LDFLAGS  += $(call ld-option,--no-warn-rwx-segments)
>  
>  KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g)
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 7b27224b5d44..d309e26aaec8 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -425,7 +425,10 @@ cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j 
> .sdata -j .data -j \
>  $(obj)/%.efi: $(obj)/%_efi.so
>   $(call cmd,efi_objcopy)
>  
> -KBUILD_EFILDFLAGS = -nostdlib -zexecstack -znocombreloc -znorelro
> +KBUILD_EFILDFLAGS = -nostdlib
> +ifeq ($(MSYS_VERSION),0)
> +KBUILD_EFILDFLAGS += -zexecstack -znocombreloc -znorelro
> +endif
>  KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments)
>  quiet_cmd_efi_ld = LD  $@
>  cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \

We should use $(call ld-option,...) here instead.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 03/31] u_boot_pylib: Make pty optional

2023-04-25 Thread Tom Rini
On Mon, Apr 24, 2023 at 05:08:08PM -0600, Simon Glass wrote:
> This library is not available on Windows. Detect this and work around it
> by using a normal pipe.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  scripts/make_pip.sh   |  9 +++--
>  tools/u_boot_pylib/cros_subprocess.py | 28 ++-
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/make_pip.sh b/scripts/make_pip.sh
> index 4602dcf61c88..bcff65240ba1 100755
> --- a/scripts/make_pip.sh
> +++ b/scripts/make_pip.sh
> @@ -91,7 +91,12 @@ find ${dest} -name __pycache__ -type f -exec rm {} \;
>  find ${dest} -depth -name __pycache__ -exec rmdir 112 \;
>  
>  # Remove test files
> -rm -rf ${dest}/*test*
> +for path in ${dest}/*test*; do
> + echo ${path}
> + if ! [[ "${path}" =~ .*test_util.* ]]; then
> + rm -rf ${path}
> + fi
> +done
>  
>  mkdir ${dir}/tests
>  cd ${dir}
> @@ -108,7 +113,7 @@ echo "Completed build of ${tool}"
>  # Use --skip-existing to work even if the version is already present
>  if [ -n "${upload}" ]; then
>   echo "Uploading from ${dir}"
> - python3 -m twine upload ${repo} -u __token__ dist/*
> + python3 -m twine upload ${repo} --verbose -u __token__ dist/*
>   echo "Completed upload of ${tool}"
>  fi

The verbose here is I assume a debugging left-over.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 2/2] .mailmap: Map all Xilinx users mail ids to AMD

2023-04-25 Thread Tom Rini
On Mon, Apr 24, 2023 at 12:44:12AM -0600, Ashok Reddy Soma wrote:
> From: Algapally Santosh Sagar 
> 
> The mail ids of all the current Xilinx users are to be mapped to AMD
> following the merger with AMD. The mailmap file is updated accordingly.
> 
> The ids of Marek Behún and Michal Simek are taken as reference.
> 
> Signed-off-by: Algapally Santosh Sagar 
> Signed-off-by: Ashok Reddy Soma 
> ---
> 
>  .mailmap | 54 +++---
>  1 file changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 80076f7206..46a8619d78 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -17,27 +17,46 @@
>  
>  Alexander Graf  
>  Allen Martin 
> +Amanda Baze  
> +Amit Kumar Mahapatra  
> 
>  Andreas Bießmann 
>  Andreas Bießmann 
>  Aneesh V 
>  Anup Patel  
> +Anurag Kumar Vulisha  
> 
> +Appana Durga Kedareswara rao  
> 
> +Ashok Reddy Soma  
>  Atish Patra  
> +Bharat Kumar Gogada  
> 
> +Bharat Kumar Gogada  

You've added a space before the '>' in the new email here.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 00/30] ide: Clean up code and fix a few bugs

2023-04-25 Thread Tom Rini
On Tue, Mar 28, 2023 at 08:06:47AM +1300, Simon Glass wrote:

> This code was converted to driver model a long time again but it was a
> pretty rough conversion. It introduced a few minor bugs, e.g. the device
> capacity is incorrect and some flags are lost (such as lba48).
> 
> This series tidies up the code and fixes these bugs. This involves quite
> a bit of refactoring, so it is done one patch at a time for easier
> review.

This doesn't apply cleanly currently, and my first attempt at applying
resulted in fails to build, please rebase and resend, thanks.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH] arm: imx8m: remove unused and obsolete board_fix_fdt() in SOC context

2023-04-25 Thread Hugo Villeneuve
From: Hugo Villeneuve 

It doesn't seem appropriate for arch/SOC to use a board-level
functionality (CONFIG_OF_BOARD_FIXUP), because this prevents boards
that need to do FDT fixup from using that feature.

Also, this code is completely dead and useless (from comments by
Rasmus Villemoes on the mailing list):

  - No in-tree imx8m-based board seems to set CONFIG_OF_BOARD_FIXUP
  - The nodes which that function wants to disable don't even exist in
the U-Boot copy of imx8mp.dtsi.

This code was introduced in commit 35bb60787b88. It seems to be some
random import of code from downstream NXP U-Boot, with a commit
message that makes no sense in upstream context.

Signed-off-by: Hugo Villeneuve 
---
 arch/arm/mach-imx/imx8m/soc.c | 34 --
 1 file changed, 34 deletions(-)

diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index df865e997d..2ec5c3dc43 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -1395,40 +1395,6 @@ usb_modify_speed:
 }
 #endif
 
-#ifdef CONFIG_OF_BOARD_FIXUP
-#ifndef CONFIG_SPL_BUILD
-int board_fix_fdt(void *fdt)
-{
-   if (is_imx8mpul()) {
-   int i = 0;
-   int nodeoff, ret;
-   const char *status = "disabled";
-   static const char * const dsi_nodes[] = {
-   "/soc@0/bus@32c0/mipi_dsi@32e6",
-   "/soc@0/bus@32c0/lcd-controller@32e8",
-   "/dsi-host"
-   };
-
-   for (i = 0; i < ARRAY_SIZE(dsi_nodes); i++) {
-   nodeoff = fdt_path_offset(fdt, dsi_nodes[i]);
-   if (nodeoff > 0) {
-set_status:
-   ret = fdt_setprop(fdt, nodeoff, "status", 
status,
- strlen(status) + 1);
-   if (ret == -FDT_ERR_NOSPACE) {
-   ret = fdt_increase_size(fdt, 512);
-   if (!ret)
-   goto set_status;
-   }
-   }
-   }
-   }
-
-   return 0;
-}
-#endif
-#endif
-
 #if !CONFIG_IS_ENABLED(SYSRESET)
 void reset_cpu(void)
 {
-- 
2.30.2



Re: Unable to implement board fdt-fixup for imx8m CPU

2023-04-25 Thread Hugo Villeneuve
On Tue, 25 Apr 2023 14:19:14 +0200
Rasmus Villemoes  wrote:

> On 25/04/2023 04.17, Hugo Villeneuve wrote:
> 
> > I need to detect at runtime the SOM configuration from an EEPROM, and based 
> > on that information adjust the ethernet PHY property inside the device tree.
> > 
> > I have already done that with success, it works great and as a temporary 
> > workaround I simply call my own function imx8m_board_fix_fdt() at the end 
> > of the function board_fix_fdt() of arch/arm/mach-imx/imx8m/soc.c.
> > 
> > But I don't think that a "board" level function should have been put in a 
> > SOC/arch source file in arch/arm/mach-imx/imx8m/soc.c in the first place. 
> > If it were not for that, there wouldn't be any need to define something new 
> > (event or other).
> > 
> > Maybe we should try to remove that board level function from 
> > arch/arm/mach-imx/imx8m/soc.c and use some other already existing mechanism 
> > for SOC-level device tree fixups?
> 
> AFAICT, you/we can just nuke that function completely.
> 
> (1) As you say, it's not appropriate for arch/SOC to provide that in the
> first place.
> (2) It's completely dead and useless code:
> (2a) No in-tree imx8m-based board seems to set CONFIG_OF_BOARD_FIXUP
> (2b) The nodes which that function wants to disable don't even exist in
> the U-Boot copy of imx8mp.dtsi.
> 
> Commit 35bb60787b88 should never have been applied in mainline U-Boot,
> it's some random import of code from downstream NXP U-Boot, with a
> commit message that makes no sense in upstream context.
> 
> Rasmus

Hi Rasmus,
I agree with your analysis, and I will submit a patch to remove that code.

Thank you.

-- 
Hugo Villeneuve


Re: [PATCH] sandbox: disable tracing before unmapping RAM

2023-04-25 Thread Pavel Skripkin

Hi Tom,

Tom Rini  says:

Simon takes sandbox patches himself and I've assigned this to him in
patchwork, thanks for submitting the patch.



Ah, sorry for noise then. I am just unfamiliar with u-boot development 
process.


Thank you!


With regards,
Pavel Skripkin


Re: [PATCH] sandbox: disable tracing before unmapping RAM

2023-04-25 Thread Tom Rini
On Tue, Apr 25, 2023 at 04:43:19PM +0300, Pavel Skripkin wrote:
> Hi Simon,
> 
> Simon Glass  says:
> > On Wed, 12 Apr 2023 at 12:55, Pavel Skripkin  wrote:
> > > 
> > > Currently doing 'reset' command in sandbox with tracing enabled causes
> > > SIGSEV
> > > 
> > > ```
> > > Hit any key to stop autoboot:  0
> > > =>
> > > =>
> > > => reset
> > > resetting ...
> > > Segmentation fault (core dumped)
> > > 
> > > ```
> > > 
> > > Tracing callback uses RAM buffer for storing tracing reports, but
> > > state_uninit() function unmaps whole RAM, which causes SIGSEV on umapped
> > > memory inside tracing subsystem.
> > > 
> > > Fix it by disabling tracing before unmapping memory
> > > 
> > > Signed-off-by: Pavel Skripkin 
> > > ---
> > >  arch/sandbox/cpu/state.c | 4 
> > >  1 file changed, 4 insertions(+)
> > 
> > Reviewed-by: Simon Glass 
> 
> Thank you for review!
> 
> +Tom, I guess?
> 
> @Tom, could you, please, pick following patch?
> 
> 
> https://lists.denx.de/pipermail/u-boot/2023-April/515192.html

Simon takes sandbox patches himself and I've assigned this to him in
patchwork, thanks for submitting the patch.

-- 
Tom


signature.asc
Description: PGP signature


  1   2   >