add support for the asus C201 chromebook (4GiB)

Add entry for GigaDevice gd25q32b part.

Signed-off-by Marty E. Plummer <[email protected]>

---
 drivers/mtd/spi/spi_flash_ids.c | 1 +
 1 file changed, 1 insertion(+)

arch/arm/dts/rk3288-veyron-speedy.dts new file +53 -0

+ */
+
+/dts-v1/;
+#include "rk3288-veyron-chromebook.dtsi"
+#include "cros-ec-sbs.dtsi"
+
+/ {
+	model = "Google Speedy";
+	compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
+		     "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
+		     "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
+		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
+		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	panel_regulator: panel-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&lcd_enable_h>;
+		regulator-name = "panel_regulator";
+		startup-delay-us = <100000>;
+		vin-supply = <&vcc33_sys>;
+	};
+
+	vcc18_lcd: vcc18-lcd {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&avdd_1v8_disp_en>;
+		regulator-name = "vcc18_lcd";
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc18_wl>;
+	};
+
+	backlight_regulator: backlight-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bl_pwr_en>;
+		regulator-name = "backlight_regulator";
+		vin-supply = <&vcc33_sys>;
+		startup-delay-us = <15000>;
+	};
+};
+

arch/arm/dts/rk3288-veyron-speedy.dts new file +119 -0

+&gpio_keys {
+	power {
+		gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&backlight {
+	power-supply = <&backlight_regulator>;
+};
+
+&panel {
+	power-supply = <&panel_regulator>;
+};
+
+&rk808 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pmic_int_l>;
+};
+
+&sdmmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+			&sdmmc_bus4>;
+	disable-wp;
+};
+
+&vcc_5v {
+	enable-active-high;
+	gpio = <&gpio7 RK_PC5 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&drv_5v>;
+};
+
+&vcc50_hdmi {
+	enable-active-high;
+	gpio = <&gpio5 RK_PC3 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&vcc50_hdmi_en>;
+};
+
+&edp {
+	pinctrl-names = "default";
+	pinctrl-0 = <&edp_hpd>;
+};
+
+&pinctrl {
+	backlight {
+		bl_pwr_en: bl_pwr_en {
+			rockchip,pins = <2 12 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	buck-5v {
+		drv_5v: drv-5v {
+			rockchip,pins = <7 21 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	edp {
+		edp_hpd: edp_hpd {
+			rockchip,pins = <7 11 RK_FUNC_2 &pcfg_pull_down>;
+		};
+	};
+
+	emmc {
+		/* Make sure eMMC is not in reset */
+		emmc_deassert_reset: emmc-deassert-reset {
+			rockchip,pins = <2 9 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	hdmi {
+		vcc50_hdmi_en: vcc50-hdmi-en {
+			rockchip,pins = <5 19 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	lcd {
+		lcd_enable_h: lcd-en {
+			rockchip,pins = <7 14 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		avdd_1v8_disp_en: avdd-1v8-disp-en {
+			rockchip,pins = <2 13 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		dvs_1: dvs-1 {
+			rockchip,pins = <7 12 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		dvs_2: dvs-2 {
+			rockchip,pins = <7 15 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&i2c4 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c4_xfer>;
+
+	trackpad at 15 {
+		compatible = "elan,i2c_touchpad";
+		interrupt-parent = <&gpio7>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		/*
+		 * Remove the inherited pinctrl settings to avoid clashing
+		 * with bus-wide ones.
+		 */
+		/delete-property/pinctrl-names;
+		/delete-property/pinctrl-0;
+		reg = <0x15>;
+		vcc-supply = <&vcc33_io>;
+		wakeup-source;
+	};
+};

arch/arm/mach-rockchip/rk3288-board-spl.c +0 -0

index f3ea624277..9ae4802647 100644
@@ -72,7 +72,8 @@ u32 spl_boot_device(void)

fallback:

#elif defined(CONFIG_TARGET_CHROMEBOOK_JERRY) || \
		defined(CONFIG_TARGET_CHROMEBIT_MICKEY) || \

arch/arm/mach-rockchip/rk3288-board-spl.c +2 -1

-		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE)
+		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE) || \
+		defined(CONFIG_TARGET_CHROMEBOOK_SPEEDY)

	return BOOT_DEVICE_SPI;
#endif
	return BOOT_DEVICE_MMC1;

arch/arm/mach-rockchip/rk3288/Kconfig +11 -0

index 6beb26fd7a..8d1d37895d 100644
@@ -30,6 +30,17 @@ config TARGET_CHROMEBOOK_MINNIE
 	  functions. It includes 2 or 4GB of SDRAM and 16 or 32GB of
 	  internal MMC. The product name is ASUS Chromebook Flip.
 
+config TARGET_CHROMEBOOK_SPEEDY
+	bool "Google/Rockchip Veyron-Speedy Chromebook"
+	select BOARD_LATE_INIT
+	help
+	  Speedy is a RK3288-based clamshell device with 2 USB 2.0 ports,
+	  micro HDMI, an 11.6 inch display, micro-SD card,
+	  HD camera, touchpad, wifi and Bluetooth. It includes a Chrome OS
+	  EC (Cortex-M3) to provide access to the keyboard and battery
+	  functions. It includes 2 or 4GB of SDRAM and 16GB of internal MMC.
+	  The product name is Asus Chromebook C201PA.
+

config TARGET_EVB_RK3288

	bool "Evb-RK3288"
	select BOARD_LATE_INIT

board/google/veyron/Kconfig +0 -0

index 770e9aad28..7f55d78dac 100644
@@ -45,3 +45,19 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	def_bool y
 

endif

board/google/veyron/Kconfig +16 -0

+
+if TARGET_CHROMEBOOK_SPEEDY
+
+config SYS_BOARD
+	default "veyron"
+
+config SYS_VENDOR
+	default "google"
+
+config SYS_CONFIG_NAME
+	default "veyron"
+
+config BOARD_SPECIFIC_OPTIONS # dummy
+	def_bool y
+
+endif

configs/chromebook_speedy_defconfig new file +96 -0

new file mode 100644
index 0000000000..8789b56f37
@@ -0,0 +1,96 @@
+CONFIG_ARM=y
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_SYS_TEXT_BASE=0x00100000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_ROCKCHIP_RK3288=y
+# CONFIG_SPL_MMC_SUPPORT is not set
+CONFIG_TARGET_CHROMEBOOK_SPEEDY=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI_SUPPORT=y
+CONFIG_SPL_STACK_R_ADDR=0x80000
+CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-speedy"
+CONFIG_DEBUG_UART=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_SILENT_CONSOLE=y
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2000
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SF_TEST=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+# CONFIG_SPL_DOS_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_SPL_PARTITION_UUIDS=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_SPL_OF_PLATDATA=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+# CONFIG_SPL_SIMPLE_BUS is not set
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_I2C_MUX=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_CROS_EC_KEYB=y
+CONFIG_CROS_EC=y
+CONFIG_CROS_EC_SPI=y
+CONFIG_PWRSEQ=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_PINCTRL=y
+CONFIG_SPL_PINCTRL=y
+# CONFIG_SPL_PINCTRL_FULL is not set
+CONFIG_PINCTRL_ROCKCHIP_RK3288=y
+CONFIG_DM_PMIC=y
+# CONFIG_SPL_PMIC_CHILDREN is not set
+CONFIG_PMIC_RK8XX=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM=y
+CONFIG_SPL_RAM=y
+CONFIG_DEBUG_UART_BASE=0xff690000
+CONFIG_DEBUG_UART_CLOCK=24000000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_ROCKCHIP_SERIAL=y
+CONFIG_ROCKCHIP_SPI=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_ROCKCHIP_USB2_PHY=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Rockchip"
+CONFIG_USB_GADGET_VENDOR_NUM=0x2207
+CONFIG_USB_GADGET_PRODUCT_NUM=0x320a
+CONFIG_USB_GADGET_DWC2_OTG=y
+CONFIG_USB_FUNCTION_MASS_STORAGE=y
+CONFIG_DM_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+CONFIG_USE_TINY_PRINTF=y
+CONFIG_CMD_DHRYSTONE=y
+CONFIG_ERRNO_STR=y
+# CONFIG_SPL_OF_LIBFDT is not set

Signature · 6 lines

2.17.0

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 33 lines · Marty E. Plummer

This adds support for the ASUS C201, a RK3288-based clamshell device. The device tree comes from linus's linux tree at 87ef12027b9b1dd0e0b12cf311fbcb19f9d92539. The SDRAM parameters are for 4GB Samsung LPDDR3, decoded from coreboot's src/mainboard/google/veyron/sdram_inf/sdram-lpddr3-samsung-4GB.inc

Signed-off-by Marty E. Plummer <[email protected]>

---

arch/arm/dts/Makefile                     |   1 +
arch/arm/dts/rk3288-veyron-speedy.dts     | 189 ++++++++++++++++++++++
arch/arm/mach-rockchip/rk3288-board-spl.c |   3 +-
arch/arm/mach-rockchip/rk3288/Kconfig     |  11 ++
board/google/veyron/Kconfig               |  16 ++
configs/chromebook_speedy_defconfig       |  96 +++++++++++
6 files changed, 315 insertions(+), 1 deletion(-)

create mode 100644 arch/arm/dts/rk3288-veyron-speedy.dts create mode 100644 configs/chromebook_speedy_defconfig

arch/arm/dts/Makefile +1 -0

index ac7667b1e8..ee04d9bedd 100644
@@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
 	rk3288-veyron-jerry.dtb \
 	rk3288-veyron-mickey.dtb \
 	rk3288-veyron-minnie.dtb \
+	rk3288-veyron-speedy.dtb \
 	rk3288-vyasa.dtb \
 	rk3328-evb.dtb \
 	rk3368-lion.dtb \

arch/arm/dts/rk3288-veyron-speedy.dts

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +0 -0

new file mode 100644
index 0000000000..d5383cef0d

This file looks quite different then the one I see on kernel.org with that revision id. So you are sure you imported that one?

arch/arm/dts/rk3288-veyron-speedy.dts line 6

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +6 -0

@@ -0,0 +1,189 @@
+/*
+ * Google Veyron Speedy Rev 1+ board device tree source
+ *
+ * Copyright 2015 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0

This file is dual licensed upstream, keep it that way. The comment will claim it's a X11 license but the license text below is actually MIT so you may want to use the MIT SPDX tag for that.

Quoted 58 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +58 -0

+ */
+
+/dts-v1/;
+#include "rk3288-veyron-chromebook.dtsi"
+#include "cros-ec-sbs.dtsi"
+
+/ {
+	model = "Google Speedy";
+	compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
+		     "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
+		     "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
+		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
+		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	panel_regulator: panel-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&lcd_enable_h>;
+		regulator-name = "panel_regulator";
+		startup-delay-us = <100000>;
+		vin-supply = <&vcc33_sys>;
+	};
+
+	vcc18_lcd: vcc18-lcd {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&avdd_1v8_disp_en>;
+		regulator-name = "vcc18_lcd";
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc18_wl>;
+	};
+
+	backlight_regulator: backlight-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bl_pwr_en>;
+		regulator-name = "backlight_regulator";
+		vin-supply = <&vcc33_sys>;
+		startup-delay-us = <15000>;
+	};
+};
+
+&dmc {
+	rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
+		0x6 0x0 0x8 0x4 0x17 0x24 0xd 0x6
+		0x4 0x8 0x4 0x76 0x4 0x0 0x30 0x0
+		0x1 0x2 0x2 0x4 0x0 0x0 0xc0 0x4

arch/arm/dts/rk3288-veyron-speedy.dts line 69

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +5 -0

+		0x8 0x1f4>;
+	rockchip,phy-timing = <0x48d7dd93 0x187008d8 0x121076
+		0x0 0xc3 0x6 0x1>;
+	rockchip,sdram-params = <0x20D266A4 0x5B6 6 533000000 6 13 0>;
+};

Not sure if this should go into a separate dtsi. One of the maintainer will have a preferred way I think, so lets see what they will add.

Quoted 288 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +119 -0

+&gpio_keys {
+	power {
+		gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&backlight {
+	power-supply = <&backlight_regulator>;
+};
+
+&panel {
+	power-supply = <&panel_regulator>;
+};
+
+&rk808 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pmic_int_l>;
+};
+
+&sdmmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+			&sdmmc_bus4>;
+	disable-wp;
+};
+
+&vcc_5v {
+	enable-active-high;
+	gpio = <&gpio7 RK_PC5 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&drv_5v>;
+};
+
+&vcc50_hdmi {
+	enable-active-high;
+	gpio = <&gpio5 RK_PC3 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&vcc50_hdmi_en>;
+};
+
+&edp {
+	pinctrl-names = "default";
+	pinctrl-0 = <&edp_hpd>;
+};
+
+&pinctrl {
+	backlight {
+		bl_pwr_en: bl_pwr_en {
+			rockchip,pins = <2 12 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	buck-5v {
+		drv_5v: drv-5v {
+			rockchip,pins = <7 21 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	edp {
+		edp_hpd: edp_hpd {
+			rockchip,pins = <7 11 RK_FUNC_2 &pcfg_pull_down>;
+		};
+	};
+
+	emmc {
+		/* Make sure eMMC is not in reset */
+		emmc_deassert_reset: emmc-deassert-reset {
+			rockchip,pins = <2 9 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	hdmi {
+		vcc50_hdmi_en: vcc50-hdmi-en {
+			rockchip,pins = <5 19 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	lcd {
+		lcd_enable_h: lcd-en {
+			rockchip,pins = <7 14 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		avdd_1v8_disp_en: avdd-1v8-disp-en {
+			rockchip,pins = <2 13 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		dvs_1: dvs-1 {
+			rockchip,pins = <7 12 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		dvs_2: dvs-2 {
+			rockchip,pins = <7 15 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&i2c4 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c4_xfer>;
+
+	trackpad at 15 {
+		compatible = "elan,i2c_touchpad";
+		interrupt-parent = <&gpio7>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		/*
+		 * Remove the inherited pinctrl settings to avoid clashing
+		 * with bus-wide ones.
+		 */
+		/delete-property/pinctrl-names;
+		/delete-property/pinctrl-0;
+		reg = <0x15>;
+		vcc-supply = <&vcc33_io>;
+		wakeup-source;
+	};
+};

arch/arm/mach-rockchip/rk3288-board-spl.c +0 -0

index f3ea624277..9ae4802647 100644
@@ -72,7 +72,8 @@ u32 spl_boot_device(void)

fallback:

#elif defined(CONFIG_TARGET_CHROMEBOOK_JERRY) || \
		defined(CONFIG_TARGET_CHROMEBIT_MICKEY) || \

arch/arm/mach-rockchip/rk3288-board-spl.c +2 -1

-		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE)
+		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE) || \
+		defined(CONFIG_TARGET_CHROMEBOOK_SPEEDY)

	return BOOT_DEVICE_SPI;
#endif
	return BOOT_DEVICE_MMC1;

arch/arm/mach-rockchip/rk3288/Kconfig +11 -0

index 6beb26fd7a..8d1d37895d 100644
@@ -30,6 +30,17 @@ config TARGET_CHROMEBOOK_MINNIE
 	  functions. It includes 2 or 4GB of SDRAM and 16 or 32GB of
 	  internal MMC. The product name is ASUS Chromebook Flip.
 
+config TARGET_CHROMEBOOK_SPEEDY
+	bool "Google/Rockchip Veyron-Speedy Chromebook"
+	select BOARD_LATE_INIT
+	help
+	  Speedy is a RK3288-based clamshell device with 2 USB 2.0 ports,
+	  micro HDMI, an 11.6 inch display, micro-SD card,
+	  HD camera, touchpad, wifi and Bluetooth. It includes a Chrome OS
+	  EC (Cortex-M3) to provide access to the keyboard and battery
+	  functions. It includes 2 or 4GB of SDRAM and 16GB of internal MMC.
+	  The product name is Asus Chromebook C201PA.
+

config TARGET_EVB_RK3288

	bool "Evb-RK3288"
	select BOARD_LATE_INIT

board/google/veyron/Kconfig +0 -0

index 770e9aad28..7f55d78dac 100644
@@ -45,3 +45,19 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	def_bool y
 

endif

board/google/veyron/Kconfig +16 -0

+
+if TARGET_CHROMEBOOK_SPEEDY
+
+config SYS_BOARD
+	default "veyron"
+
+config SYS_VENDOR
+	default "google"
+
+config SYS_CONFIG_NAME
+	default "veyron"
+
+config BOARD_SPECIFIC_OPTIONS # dummy
+	def_bool y
+
+endif

configs/chromebook_speedy_defconfig new file +96 -0

new file mode 100644
index 0000000000..8789b56f37
@@ -0,0 +1,96 @@
+CONFIG_ARM=y
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_SYS_TEXT_BASE=0x00100000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_ROCKCHIP_RK3288=y
+# CONFIG_SPL_MMC_SUPPORT is not set
+CONFIG_TARGET_CHROMEBOOK_SPEEDY=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI_SUPPORT=y
+CONFIG_SPL_STACK_R_ADDR=0x80000
+CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-speedy"
+CONFIG_DEBUG_UART=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_SILENT_CONSOLE=y
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2000
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SF_TEST=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+# CONFIG_SPL_DOS_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_SPL_PARTITION_UUIDS=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_SPL_OF_PLATDATA=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+# CONFIG_SPL_SIMPLE_BUS is not set
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_I2C_MUX=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_CROS_EC_KEYB=y
+CONFIG_CROS_EC=y
+CONFIG_CROS_EC_SPI=y
+CONFIG_PWRSEQ=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_PINCTRL=y
+CONFIG_SPL_PINCTRL=y
+# CONFIG_SPL_PINCTRL_FULL is not set
+CONFIG_PINCTRL_ROCKCHIP_RK3288=y
+CONFIG_DM_PMIC=y
+# CONFIG_SPL_PMIC_CHILDREN is not set
+CONFIG_PMIC_RK8XX=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM=y
+CONFIG_SPL_RAM=y
+CONFIG_DEBUG_UART_BASE=0xff690000
+CONFIG_DEBUG_UART_CLOCK=24000000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_ROCKCHIP_SERIAL=y
+CONFIG_ROCKCHIP_SPI=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_ROCKCHIP_USB2_PHY=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Rockchip"
+CONFIG_USB_GADGET_VENDOR_NUM=0x2207
+CONFIG_USB_GADGET_PRODUCT_NUM=0x320a
+CONFIG_USB_GADGET_DWC2_OTG=y
+CONFIG_USB_FUNCTION_MASS_STORAGE=y
+CONFIG_DM_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+CONFIG_USE_TINY_PRINTF=y
+CONFIG_CMD_DHRYSTONE=y
+CONFIG_ERRNO_STR=y
+# CONFIG_SPL_OF_LIBFDT is not set

Signature · 6 lines

2.17.0

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 33 lines · klaus.goger at theobroma-systems.com

Quoted 33 lines · Marty E. Plummer

This adds support for the ASUS C201, a RK3288-based clamshell device. The device tree comes from linus's linux tree at 87ef12027b9b1dd0e0b12cf311fbcb19f9d92539. The SDRAM parameters are for 4GB Samsung LPDDR3, decoded from coreboot's src/mainboard/google/veyron/sdram_inf/sdram-lpddr3-samsung-4GB.inc

Signed-off-by Marty E. Plummer <[email protected]>

---

arch/arm/dts/Makefile                     |   1 +
arch/arm/dts/rk3288-veyron-speedy.dts     | 189 ++++++++++++++++++++++
arch/arm/mach-rockchip/rk3288-board-spl.c |   3 +-
arch/arm/mach-rockchip/rk3288/Kconfig     |  11 ++
board/google/veyron/Kconfig               |  16 ++
configs/chromebook_speedy_defconfig       |  96 +++++++++++
6 files changed, 315 insertions(+), 1 deletion(-)

create mode 100644 arch/arm/dts/rk3288-veyron-speedy.dts create mode 100644 configs/chromebook_speedy_defconfig

arch/arm/dts/Makefile +1 -0

index ac7667b1e8..ee04d9bedd 100644
@@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
 	rk3288-veyron-jerry.dtb \
 	rk3288-veyron-mickey.dtb \
 	rk3288-veyron-minnie.dtb \
+	rk3288-veyron-speedy.dtb \
 	rk3288-vyasa.dtb \
 	rk3328-evb.dtb \
 	rk3368-lion.dtb \

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +0 -0

new file mode 100644
index 0000000000..d5383cef0d

This file looks quite different then the one I see on kernel.org with that revision id. So you are sure you imported that one?

Dafuq... it seems I borked something up in doing the patch juggling to turn my single-commit mess of a patch (you know, the 'get the thing to work branch') into a good patch series I messed up on this one.

Quoted 13 lines

arch/arm/dts/rk3288-veyron-speedy.dts line 6

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +6 -0

@@ -0,0 +1,189 @@
+/*
+ * Google Veyron Speedy Rev 1+ board device tree source
+ *
+ * Copyright 2015 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0

This file is dual licensed upstream, keep it that way. The comment will claim it's a X11 license but the license text below is actually MIT so you may want to use the MIT SPDX tag for that.

Yeah, I was listening in on the convo on irc. So, even though it 'says' its GPL/X11, the actual license text is MIT, so I should use that tag? Its not my code, obviously, so I have no dog in that race anyways.

Quoted 58 lines

Quoted 58 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +58 -0

+ */
+
+/dts-v1/;
+#include "rk3288-veyron-chromebook.dtsi"
+#include "cros-ec-sbs.dtsi"
+
+/ {
+	model = "Google Speedy";
+	compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
+		     "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
+		     "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
+		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
+		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	panel_regulator: panel-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&lcd_enable_h>;
+		regulator-name = "panel_regulator";
+		startup-delay-us = <100000>;
+		vin-supply = <&vcc33_sys>;
+	};
+
+	vcc18_lcd: vcc18-lcd {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&avdd_1v8_disp_en>;
+		regulator-name = "vcc18_lcd";
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc18_wl>;
+	};
+
+	backlight_regulator: backlight-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bl_pwr_en>;
+		regulator-name = "backlight_regulator";
+		vin-supply = <&vcc33_sys>;
+		startup-delay-us = <15000>;
+	};
+};
+
+&dmc {
+	rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
+		0x6 0x0 0x8 0x4 0x17 0x24 0xd 0x6
+		0x4 0x8 0x4 0x76 0x4 0x0 0x30 0x0
+		0x1 0x2 0x2 0x4 0x0 0x0 0xc0 0x4

Quoted 9 lines

arch/arm/dts/rk3288-veyron-speedy.dts line 69

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +5 -0

+		0x8 0x1f4>;
+	rockchip,phy-timing = <0x48d7dd93 0x187008d8 0x121076
+		0x0 0xc3 0x6 0x1>;
+	rockchip,sdram-params = <0x20D266A4 0x5B6 6 533000000 6 13 0>;
+};

Not sure if this should go into a separate dtsi. One of the maintainer will have a preferred way I think, so lets see what they will add.

Yeah, that is possible. However, here I just followed the example of rk3288-veyron-jerry.dtsi, which I (perhaps wrongly) assume can come in other memory size variants.

Quoted 284 lines

Quoted 284 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +119 -0

+&gpio_keys {
+	power {
+		gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&backlight {
+	power-supply = <&backlight_regulator>;
+};
+
+&panel {
+	power-supply = <&panel_regulator>;
+};
+
+&rk808 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pmic_int_l>;
+};
+
+&sdmmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+			&sdmmc_bus4>;
+	disable-wp;
+};
+
+&vcc_5v {
+	enable-active-high;
+	gpio = <&gpio7 RK_PC5 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&drv_5v>;
+};
+
+&vcc50_hdmi {
+	enable-active-high;
+	gpio = <&gpio5 RK_PC3 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&vcc50_hdmi_en>;
+};
+
+&edp {
+	pinctrl-names = "default";
+	pinctrl-0 = <&edp_hpd>;
+};
+
+&pinctrl {
+	backlight {
+		bl_pwr_en: bl_pwr_en {
+			rockchip,pins = <2 12 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	buck-5v {
+		drv_5v: drv-5v {
+			rockchip,pins = <7 21 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	edp {
+		edp_hpd: edp_hpd {
+			rockchip,pins = <7 11 RK_FUNC_2 &pcfg_pull_down>;
+		};
+	};
+
+	emmc {
+		/* Make sure eMMC is not in reset */
+		emmc_deassert_reset: emmc-deassert-reset {
+			rockchip,pins = <2 9 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	hdmi {
+		vcc50_hdmi_en: vcc50-hdmi-en {
+			rockchip,pins = <5 19 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	lcd {
+		lcd_enable_h: lcd-en {
+			rockchip,pins = <7 14 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		avdd_1v8_disp_en: avdd-1v8-disp-en {
+			rockchip,pins = <2 13 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		dvs_1: dvs-1 {
+			rockchip,pins = <7 12 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		dvs_2: dvs-2 {
+			rockchip,pins = <7 15 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&i2c4 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c4_xfer>;
+
+	trackpad at 15 {
+		compatible = "elan,i2c_touchpad";
+		interrupt-parent = <&gpio7>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		/*
+		 * Remove the inherited pinctrl settings to avoid clashing
+		 * with bus-wide ones.
+		 */
+		/delete-property/pinctrl-names;
+		/delete-property/pinctrl-0;
+		reg = <0x15>;
+		vcc-supply = <&vcc33_io>;
+		wakeup-source;
+	};
+};

arch/arm/mach-rockchip/rk3288-board-spl.c +0 -0

index f3ea624277..9ae4802647 100644
@@ -72,7 +72,8 @@ u32 spl_boot_device(void)

fallback:

#elif defined(CONFIG_TARGET_CHROMEBOOK_JERRY) || \
		defined(CONFIG_TARGET_CHROMEBIT_MICKEY) || \

arch/arm/mach-rockchip/rk3288-board-spl.c +2 -1

-		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE)
+		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE) || \
+		defined(CONFIG_TARGET_CHROMEBOOK_SPEEDY)

	return BOOT_DEVICE_SPI;
#endif
	return BOOT_DEVICE_MMC1;

arch/arm/mach-rockchip/rk3288/Kconfig +11 -0

index 6beb26fd7a..8d1d37895d 100644
@@ -30,6 +30,17 @@ config TARGET_CHROMEBOOK_MINNIE
 	  functions. It includes 2 or 4GB of SDRAM and 16 or 32GB of
 	  internal MMC. The product name is ASUS Chromebook Flip.
 
+config TARGET_CHROMEBOOK_SPEEDY
+	bool "Google/Rockchip Veyron-Speedy Chromebook"
+	select BOARD_LATE_INIT
+	help
+	  Speedy is a RK3288-based clamshell device with 2 USB 2.0 ports,
+	  micro HDMI, an 11.6 inch display, micro-SD card,
+	  HD camera, touchpad, wifi and Bluetooth. It includes a Chrome OS
+	  EC (Cortex-M3) to provide access to the keyboard and battery
+	  functions. It includes 2 or 4GB of SDRAM and 16GB of internal MMC.
+	  The product name is Asus Chromebook C201PA.
+

config TARGET_EVB_RK3288

	bool "Evb-RK3288"
	select BOARD_LATE_INIT

board/google/veyron/Kconfig +0 -0

index 770e9aad28..7f55d78dac 100644
@@ -45,3 +45,19 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	def_bool y
 

endif

board/google/veyron/Kconfig +16 -0

+
+if TARGET_CHROMEBOOK_SPEEDY
+
+config SYS_BOARD
+	default "veyron"
+
+config SYS_VENDOR
+	default "google"
+
+config SYS_CONFIG_NAME
+	default "veyron"
+
+config BOARD_SPECIFIC_OPTIONS # dummy
+	def_bool y
+
+endif

configs/chromebook_speedy_defconfig new file +96 -0

new file mode 100644
index 0000000000..8789b56f37
@@ -0,0 +1,96 @@
+CONFIG_ARM=y
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_SYS_TEXT_BASE=0x00100000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_ROCKCHIP_RK3288=y
+# CONFIG_SPL_MMC_SUPPORT is not set
+CONFIG_TARGET_CHROMEBOOK_SPEEDY=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI_SUPPORT=y
+CONFIG_SPL_STACK_R_ADDR=0x80000
+CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-speedy"
+CONFIG_DEBUG_UART=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_SILENT_CONSOLE=y
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2000
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SF_TEST=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+# CONFIG_SPL_DOS_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_SPL_PARTITION_UUIDS=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_SPL_OF_PLATDATA=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+# CONFIG_SPL_SIMPLE_BUS is not set
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_I2C_MUX=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_CROS_EC_KEYB=y
+CONFIG_CROS_EC=y
+CONFIG_CROS_EC_SPI=y
+CONFIG_PWRSEQ=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_PINCTRL=y
+CONFIG_SPL_PINCTRL=y
+# CONFIG_SPL_PINCTRL_FULL is not set
+CONFIG_PINCTRL_ROCKCHIP_RK3288=y
+CONFIG_DM_PMIC=y
+# CONFIG_SPL_PMIC_CHILDREN is not set
+CONFIG_PMIC_RK8XX=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM=y
+CONFIG_SPL_RAM=y
+CONFIG_DEBUG_UART_BASE=0xff690000
+CONFIG_DEBUG_UART_CLOCK=24000000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_ROCKCHIP_SERIAL=y
+CONFIG_ROCKCHIP_SPI=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_ROCKCHIP_USB2_PHY=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Rockchip"
+CONFIG_USB_GADGET_VENDOR_NUM=0x2207
+CONFIG_USB_GADGET_PRODUCT_NUM=0x320a
+CONFIG_USB_GADGET_DWC2_OTG=y
+CONFIG_USB_FUNCTION_MASS_STORAGE=y
+CONFIG_DM_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+CONFIG_USE_TINY_PRINTF=y
+CONFIG_CMD_DHRYSTONE=y
+CONFIG_ERRNO_STR=y
+# CONFIG_SPL_OF_LIBFDT is not set

Signature · 1 line

2.17.0

Quoted 5 lines

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 131 lines · Marty E. Plummer

Quoted 33 lines · klaus.goger at theobroma-systems.com

Quoted 33 lines · Marty E. Plummer

This adds support for the ASUS C201, a RK3288-based clamshell device. The device tree comes from linus's linux tree at 87ef12027b9b1dd0e0b12cf311fbcb19f9d92539. The SDRAM parameters are for 4GB Samsung LPDDR3, decoded from coreboot's src/mainboard/google/veyron/sdram_inf/sdram-lpddr3-samsung-4GB.inc

Signed-off-by Marty E. Plummer <[email protected]>

---

arch/arm/dts/Makefile                     |   1 +
arch/arm/dts/rk3288-veyron-speedy.dts     | 189 ++++++++++++++++++++++
arch/arm/mach-rockchip/rk3288-board-spl.c |   3 +-
arch/arm/mach-rockchip/rk3288/Kconfig     |  11 ++
board/google/veyron/Kconfig               |  16 ++
configs/chromebook_speedy_defconfig       |  96 +++++++++++
6 files changed, 315 insertions(+), 1 deletion(-)

create mode 100644 arch/arm/dts/rk3288-veyron-speedy.dts create mode 100644 configs/chromebook_speedy_defconfig

arch/arm/dts/Makefile +1 -0

index ac7667b1e8..ee04d9bedd 100644
@@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
 	rk3288-veyron-jerry.dtb \
 	rk3288-veyron-mickey.dtb \
 	rk3288-veyron-minnie.dtb \
+	rk3288-veyron-speedy.dtb \
 	rk3288-vyasa.dtb \
 	rk3328-evb.dtb \
 	rk3368-lion.dtb \

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +0 -0

new file mode 100644
index 0000000000..d5383cef0d

This file looks quite different then the one I see on kernel.org with that revision id. So you are sure you imported that one?

Dafuq... it seems I borked something up in doing the patch juggling to turn my single-commit mess of a patch (you know, the 'get the thing to work branch') into a good patch series I messed up on this one.

Quoted 13 lines

arch/arm/dts/rk3288-veyron-speedy.dts line 6

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +6 -0

@@ -0,0 +1,189 @@
+/*
+ * Google Veyron Speedy Rev 1+ board device tree source
+ *
+ * Copyright 2015 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0

This file is dual licensed upstream, keep it that way. The comment will claim it's a X11 license but the license text below is actually MIT so you may want to use the MIT SPDX tag for that.

Yeah, I was listening in on the convo on irc. So, even though it 'says' its GPL/X11, the actual license text is MIT, so I should use that tag? Its not my code, obviously, so I have no dog in that race anyways.

Quoted 67 lines

Quoted 53 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +53 -0

+ */
+
+/dts-v1/;
+#include "rk3288-veyron-chromebook.dtsi"
+#include "cros-ec-sbs.dtsi"
+
+/ {
+	model = "Google Speedy";
+	compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
+		     "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
+		     "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
+		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
+		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	panel_regulator: panel-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&lcd_enable_h>;
+		regulator-name = "panel_regulator";
+		startup-delay-us = <100000>;
+		vin-supply = <&vcc33_sys>;
+	};
+
+	vcc18_lcd: vcc18-lcd {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&avdd_1v8_disp_en>;
+		regulator-name = "vcc18_lcd";
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc18_wl>;
+	};
+
+	backlight_regulator: backlight-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bl_pwr_en>;
+		regulator-name = "backlight_regulator";
+		vin-supply = <&vcc33_sys>;
+		startup-delay-us = <15000>;
+	};
+};
+

arch/arm/dts/rk3288-veyron-speedy.dts line 69

Quoted 10 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +10 -0

+&dmc {
+	rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
+		0x6 0x0 0x8 0x4 0x17 0x24 0xd 0x6
+		0x4 0x8 0x4 0x76 0x4 0x0 0x30 0x0
+		0x1 0x2 0x2 0x4 0x0 0x0 0xc0 0x4
+		0x8 0x1f4>;
+	rockchip,phy-timing = <0x48d7dd93 0x187008d8 0x121076
+		0x0 0xc3 0x6 0x1>;
+	rockchip,sdram-params = <0x20D266A4 0x5B6 6 533000000 6 13 0>;
+};

Not sure if this should go into a separate dtsi. One of the maintainer will have a preferred way I think, so lets see what they will add.

Quoted 3 lines

Yeah, that is possible. However, here I just followed the example of rk3288-veyron-jerry.dtsi, which I (perhaps wrongly) assume can come in other memory size variants.

Having a “-u-boot.dtsi” is the current practice (counting 85 such files as of today). We should follow this model here as well for any new devices we add.

Quoted 293 lines

Quoted 284 lines

Quoted 284 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +119 -0

+&gpio_keys {
+	power {
+		gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&backlight {
+	power-supply = <&backlight_regulator>;
+};
+
+&panel {
+	power-supply = <&panel_regulator>;
+};
+
+&rk808 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pmic_int_l>;
+};
+
+&sdmmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+			&sdmmc_bus4>;
+	disable-wp;
+};
+
+&vcc_5v {
+	enable-active-high;
+	gpio = <&gpio7 RK_PC5 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&drv_5v>;
+};
+
+&vcc50_hdmi {
+	enable-active-high;
+	gpio = <&gpio5 RK_PC3 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&vcc50_hdmi_en>;
+};
+
+&edp {
+	pinctrl-names = "default";
+	pinctrl-0 = <&edp_hpd>;
+};
+
+&pinctrl {
+	backlight {
+		bl_pwr_en: bl_pwr_en {
+			rockchip,pins = <2 12 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	buck-5v {
+		drv_5v: drv-5v {
+			rockchip,pins = <7 21 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	edp {
+		edp_hpd: edp_hpd {
+			rockchip,pins = <7 11 RK_FUNC_2 &pcfg_pull_down>;
+		};
+	};
+
+	emmc {
+		/* Make sure eMMC is not in reset */
+		emmc_deassert_reset: emmc-deassert-reset {
+			rockchip,pins = <2 9 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	hdmi {
+		vcc50_hdmi_en: vcc50-hdmi-en {
+			rockchip,pins = <5 19 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	lcd {
+		lcd_enable_h: lcd-en {
+			rockchip,pins = <7 14 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		avdd_1v8_disp_en: avdd-1v8-disp-en {
+			rockchip,pins = <2 13 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		dvs_1: dvs-1 {
+			rockchip,pins = <7 12 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		dvs_2: dvs-2 {
+			rockchip,pins = <7 15 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&i2c4 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c4_xfer>;
+
+	trackpad at 15 {
+		compatible = "elan,i2c_touchpad";
+		interrupt-parent = <&gpio7>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		/*
+		 * Remove the inherited pinctrl settings to avoid clashing
+		 * with bus-wide ones.
+		 */
+		/delete-property/pinctrl-names;
+		/delete-property/pinctrl-0;
+		reg = <0x15>;
+		vcc-supply = <&vcc33_io>;
+		wakeup-source;
+	};
+};

arch/arm/mach-rockchip/rk3288-board-spl.c +0 -0

index f3ea624277..9ae4802647 100644
@@ -72,7 +72,8 @@ u32 spl_boot_device(void)

fallback:

#elif defined(CONFIG_TARGET_CHROMEBOOK_JERRY) || \
		defined(CONFIG_TARGET_CHROMEBIT_MICKEY) || \

arch/arm/mach-rockchip/rk3288-board-spl.c +2 -1

-		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE)
+		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE) || \
+		defined(CONFIG_TARGET_CHROMEBOOK_SPEEDY)

	return BOOT_DEVICE_SPI;
#endif
	return BOOT_DEVICE_MMC1;

arch/arm/mach-rockchip/rk3288/Kconfig +11 -0

index 6beb26fd7a..8d1d37895d 100644
@@ -30,6 +30,17 @@ config TARGET_CHROMEBOOK_MINNIE
 	  functions. It includes 2 or 4GB of SDRAM and 16 or 32GB of
 	  internal MMC. The product name is ASUS Chromebook Flip.
 
+config TARGET_CHROMEBOOK_SPEEDY
+	bool "Google/Rockchip Veyron-Speedy Chromebook"
+	select BOARD_LATE_INIT
+	help
+	  Speedy is a RK3288-based clamshell device with 2 USB 2.0 ports,
+	  micro HDMI, an 11.6 inch display, micro-SD card,
+	  HD camera, touchpad, wifi and Bluetooth. It includes a Chrome OS
+	  EC (Cortex-M3) to provide access to the keyboard and battery
+	  functions. It includes 2 or 4GB of SDRAM and 16GB of internal MMC.
+	  The product name is Asus Chromebook C201PA.
+

config TARGET_EVB_RK3288

	bool "Evb-RK3288"
	select BOARD_LATE_INIT

board/google/veyron/Kconfig +0 -0

index 770e9aad28..7f55d78dac 100644
@@ -45,3 +45,19 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	def_bool y
 

endif

board/google/veyron/Kconfig +16 -0

+
+if TARGET_CHROMEBOOK_SPEEDY
+
+config SYS_BOARD
+	default "veyron"
+
+config SYS_VENDOR
+	default "google"
+
+config SYS_CONFIG_NAME
+	default "veyron"
+
+config BOARD_SPECIFIC_OPTIONS # dummy
+	def_bool y
+
+endif

configs/chromebook_speedy_defconfig new file +96 -0

new file mode 100644
index 0000000000..8789b56f37
@@ -0,0 +1,96 @@
+CONFIG_ARM=y
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_SYS_TEXT_BASE=0x00100000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_ROCKCHIP_RK3288=y
+# CONFIG_SPL_MMC_SUPPORT is not set
+CONFIG_TARGET_CHROMEBOOK_SPEEDY=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI_SUPPORT=y
+CONFIG_SPL_STACK_R_ADDR=0x80000
+CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-speedy"
+CONFIG_DEBUG_UART=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_SILENT_CONSOLE=y
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2000
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SF_TEST=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+# CONFIG_SPL_DOS_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_SPL_PARTITION_UUIDS=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_SPL_OF_PLATDATA=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+# CONFIG_SPL_SIMPLE_BUS is not set
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_I2C_MUX=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_CROS_EC_KEYB=y
+CONFIG_CROS_EC=y
+CONFIG_CROS_EC_SPI=y
+CONFIG_PWRSEQ=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_PINCTRL=y
+CONFIG_SPL_PINCTRL=y
+# CONFIG_SPL_PINCTRL_FULL is not set
+CONFIG_PINCTRL_ROCKCHIP_RK3288=y
+CONFIG_DM_PMIC=y
+# CONFIG_SPL_PMIC_CHILDREN is not set
+CONFIG_PMIC_RK8XX=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM=y
+CONFIG_SPL_RAM=y
+CONFIG_DEBUG_UART_BASE=0xff690000
+CONFIG_DEBUG_UART_CLOCK=24000000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_ROCKCHIP_SERIAL=y
+CONFIG_ROCKCHIP_SPI=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_ROCKCHIP_USB2_PHY=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Rockchip"
+CONFIG_USB_GADGET_VENDOR_NUM=0x2207
+CONFIG_USB_GADGET_PRODUCT_NUM=0x320a
+CONFIG_USB_GADGET_DWC2_OTG=y
+CONFIG_USB_FUNCTION_MASS_STORAGE=y
+CONFIG_DM_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+CONFIG_USE_TINY_PRINTF=y
+CONFIG_CMD_DHRYSTONE=y
+CONFIG_ERRNO_STR=y
+# CONFIG_SPL_OF_LIBFDT is not set

Signature · 1 line

2.17.0

Quoted 5 lines

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 135 lines · Dr. Philipp Tomsich

Quoted 134 lines · Marty E. Plummer

Quoted 33 lines · klaus.goger at theobroma-systems.com

Quoted 33 lines · Marty E. Plummer

This adds support for the ASUS C201, a RK3288-based clamshell device. The device tree comes from linus's linux tree at 87ef12027b9b1dd0e0b12cf311fbcb19f9d92539. The SDRAM parameters are for 4GB Samsung LPDDR3, decoded from coreboot's src/mainboard/google/veyron/sdram_inf/sdram-lpddr3-samsung-4GB.inc

Signed-off-by Marty E. Plummer <[email protected]>

---

arch/arm/dts/Makefile                     |   1 +
arch/arm/dts/rk3288-veyron-speedy.dts     | 189 ++++++++++++++++++++++
arch/arm/mach-rockchip/rk3288-board-spl.c |   3 +-
arch/arm/mach-rockchip/rk3288/Kconfig     |  11 ++
board/google/veyron/Kconfig               |  16 ++
configs/chromebook_speedy_defconfig       |  96 +++++++++++
6 files changed, 315 insertions(+), 1 deletion(-)

create mode 100644 arch/arm/dts/rk3288-veyron-speedy.dts create mode 100644 configs/chromebook_speedy_defconfig

arch/arm/dts/Makefile +1 -0

index ac7667b1e8..ee04d9bedd 100644
@@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
 	rk3288-veyron-jerry.dtb \
 	rk3288-veyron-mickey.dtb \
 	rk3288-veyron-minnie.dtb \
+	rk3288-veyron-speedy.dtb \
 	rk3288-vyasa.dtb \
 	rk3328-evb.dtb \
 	rk3368-lion.dtb \

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +0 -0

new file mode 100644
index 0000000000..d5383cef0d

This file looks quite different then the one I see on kernel.org with that revision id. So you are sure you imported that one?

Dafuq... it seems I borked something up in doing the patch juggling to turn my single-commit mess of a patch (you know, the 'get the thing to work branch') into a good patch series I messed up on this one.

Quoted 13 lines

arch/arm/dts/rk3288-veyron-speedy.dts line 6

Quoted 8 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +6 -0

@@ -0,0 +1,189 @@
+/*
+ * Google Veyron Speedy Rev 1+ board device tree source
+ *
+ * Copyright 2015 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0

This file is dual licensed upstream, keep it that way. The comment will claim it's a X11 license but the license text below is actually MIT so you may want to use the MIT SPDX tag for that.

Yeah, I was listening in on the convo on irc. So, even though it 'says' its GPL/X11, the actual license text is MIT, so I should use that tag? Its not my code, obviously, so I have no dog in that race anyways.

Quoted 58 lines

Quoted 58 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +58 -0

+ */
+
+/dts-v1/;
+#include "rk3288-veyron-chromebook.dtsi"
+#include "cros-ec-sbs.dtsi"
+
+/ {
+	model = "Google Speedy";
+	compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
+		     "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
+		     "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
+		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
+		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
+
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	panel_regulator: panel-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&lcd_enable_h>;
+		regulator-name = "panel_regulator";
+		startup-delay-us = <100000>;
+		vin-supply = <&vcc33_sys>;
+	};
+
+	vcc18_lcd: vcc18-lcd {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&avdd_1v8_disp_en>;
+		regulator-name = "vcc18_lcd";
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc18_wl>;
+	};
+
+	backlight_regulator: backlight-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bl_pwr_en>;
+		regulator-name = "backlight_regulator";
+		vin-supply = <&vcc33_sys>;
+		startup-delay-us = <15000>;
+	};
+};
+
+&dmc {
+	rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
+		0x6 0x0 0x8 0x4 0x17 0x24 0xd 0x6
+		0x4 0x8 0x4 0x76 0x4 0x0 0x30 0x0
+		0x1 0x2 0x2 0x4 0x0 0x0 0xc0 0x4

Quoted 9 lines

arch/arm/dts/rk3288-veyron-speedy.dts line 69

Quoted 5 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +5 -0

+		0x8 0x1f4>;
+	rockchip,phy-timing = <0x48d7dd93 0x187008d8 0x121076
+		0x0 0xc3 0x6 0x1>;
+	rockchip,sdram-params = <0x20D266A4 0x5B6 6 533000000 6 13 0>;
+};

Not sure if this should go into a separate dtsi. One of the maintainer will have a preferred way I think, so lets see what they will add.

Yeah, that is possible. However, here I just followed the example of rk3288-veyron-jerry.dtsi, which I (perhaps wrongly) assume can come in other memory size variants.

Quoted 3 lines

Having a “-u-boot.dtsi” is the current practice (counting 85 such files as of today). We should follow this model here as well for any new devices we add.

Fair. So what should go into the -u-boot.dtsi file? everything not standard to the imported dts/dtsi from linux?

Quoted 293 lines

Quoted 293 lines

Quoted 284 lines

Quoted 284 lines

arch/arm/dts/rk3288-veyron-speedy.dts new file +119 -0

+&gpio_keys {
+	power {
+		gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&backlight {
+	power-supply = <&backlight_regulator>;
+};
+
+&panel {
+	power-supply = <&panel_regulator>;
+};
+
+&rk808 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pmic_int_l>;
+};
+
+&sdmmc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd_disabled &sdmmc_cd_gpio
+			&sdmmc_bus4>;
+	disable-wp;
+};
+
+&vcc_5v {
+	enable-active-high;
+	gpio = <&gpio7 RK_PC5 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&drv_5v>;
+};
+
+&vcc50_hdmi {
+	enable-active-high;
+	gpio = <&gpio5 RK_PC3 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&vcc50_hdmi_en>;
+};
+
+&edp {
+	pinctrl-names = "default";
+	pinctrl-0 = <&edp_hpd>;
+};
+
+&pinctrl {
+	backlight {
+		bl_pwr_en: bl_pwr_en {
+			rockchip,pins = <2 12 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	buck-5v {
+		drv_5v: drv-5v {
+			rockchip,pins = <7 21 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	edp {
+		edp_hpd: edp_hpd {
+			rockchip,pins = <7 11 RK_FUNC_2 &pcfg_pull_down>;
+		};
+	};
+
+	emmc {
+		/* Make sure eMMC is not in reset */
+		emmc_deassert_reset: emmc-deassert-reset {
+			rockchip,pins = <2 9 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	hdmi {
+		vcc50_hdmi_en: vcc50-hdmi-en {
+			rockchip,pins = <5 19 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	lcd {
+		lcd_enable_h: lcd-en {
+			rockchip,pins = <7 14 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		avdd_1v8_disp_en: avdd-1v8-disp-en {
+			rockchip,pins = <2 13 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		dvs_1: dvs-1 {
+			rockchip,pins = <7 12 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		dvs_2: dvs-2 {
+			rockchip,pins = <7 15 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+};
+
+&i2c4 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c4_xfer>;
+
+	trackpad at 15 {
+		compatible = "elan,i2c_touchpad";
+		interrupt-parent = <&gpio7>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		/*
+		 * Remove the inherited pinctrl settings to avoid clashing
+		 * with bus-wide ones.
+		 */
+		/delete-property/pinctrl-names;
+		/delete-property/pinctrl-0;
+		reg = <0x15>;
+		vcc-supply = <&vcc33_io>;
+		wakeup-source;
+	};
+};

arch/arm/mach-rockchip/rk3288-board-spl.c +0 -0

index f3ea624277..9ae4802647 100644
@@ -72,7 +72,8 @@ u32 spl_boot_device(void)

fallback:

#elif defined(CONFIG_TARGET_CHROMEBOOK_JERRY) || \
		defined(CONFIG_TARGET_CHROMEBIT_MICKEY) || \

arch/arm/mach-rockchip/rk3288-board-spl.c +2 -1

-		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE)
+		defined(CONFIG_TARGET_CHROMEBOOK_MINNIE) || \
+		defined(CONFIG_TARGET_CHROMEBOOK_SPEEDY)

	return BOOT_DEVICE_SPI;
#endif
	return BOOT_DEVICE_MMC1;

arch/arm/mach-rockchip/rk3288/Kconfig +11 -0

index 6beb26fd7a..8d1d37895d 100644
@@ -30,6 +30,17 @@ config TARGET_CHROMEBOOK_MINNIE
 	  functions. It includes 2 or 4GB of SDRAM and 16 or 32GB of
 	  internal MMC. The product name is ASUS Chromebook Flip.
 
+config TARGET_CHROMEBOOK_SPEEDY
+	bool "Google/Rockchip Veyron-Speedy Chromebook"
+	select BOARD_LATE_INIT
+	help
+	  Speedy is a RK3288-based clamshell device with 2 USB 2.0 ports,
+	  micro HDMI, an 11.6 inch display, micro-SD card,
+	  HD camera, touchpad, wifi and Bluetooth. It includes a Chrome OS
+	  EC (Cortex-M3) to provide access to the keyboard and battery
+	  functions. It includes 2 or 4GB of SDRAM and 16GB of internal MMC.
+	  The product name is Asus Chromebook C201PA.
+

config TARGET_EVB_RK3288

	bool "Evb-RK3288"
	select BOARD_LATE_INIT

board/google/veyron/Kconfig +0 -0

index 770e9aad28..7f55d78dac 100644
@@ -45,3 +45,19 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	def_bool y
 

endif

board/google/veyron/Kconfig +16 -0

+
+if TARGET_CHROMEBOOK_SPEEDY
+
+config SYS_BOARD
+	default "veyron"
+
+config SYS_VENDOR
+	default "google"
+
+config SYS_CONFIG_NAME
+	default "veyron"
+
+config BOARD_SPECIFIC_OPTIONS # dummy
+	def_bool y
+
+endif

configs/chromebook_speedy_defconfig new file +96 -0

new file mode 100644
index 0000000000..8789b56f37
@@ -0,0 +1,96 @@
+CONFIG_ARM=y
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_SYS_TEXT_BASE=0x00100000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_ROCKCHIP_RK3288=y
+# CONFIG_SPL_MMC_SUPPORT is not set
+CONFIG_TARGET_CHROMEBOOK_SPEEDY=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI_SUPPORT=y
+CONFIG_SPL_STACK_R_ADDR=0x80000
+CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-speedy"
+CONFIG_DEBUG_UART=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_SILENT_CONSOLE=y
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2000
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SF_TEST=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+# CONFIG_SPL_DOS_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_SPL_PARTITION_UUIDS=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_SPL_OF_PLATDATA=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+# CONFIG_SPL_SIMPLE_BUS is not set
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_ROCKCHIP_GPIO=y
+CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_SYS_I2C_ROCKCHIP=y
+CONFIG_I2C_MUX=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_CROS_EC_KEYB=y
+CONFIG_CROS_EC=y
+CONFIG_CROS_EC_SPI=y
+CONFIG_PWRSEQ=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_ROCKCHIP=y
+CONFIG_PINCTRL=y
+CONFIG_SPL_PINCTRL=y
+# CONFIG_SPL_PINCTRL_FULL is not set
+CONFIG_PINCTRL_ROCKCHIP_RK3288=y
+CONFIG_DM_PMIC=y
+# CONFIG_SPL_PMIC_CHILDREN is not set
+CONFIG_PMIC_RK8XX=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_ROCKCHIP=y
+CONFIG_RAM=y
+CONFIG_SPL_RAM=y
+CONFIG_DEBUG_UART_BASE=0xff690000
+CONFIG_DEBUG_UART_CLOCK=24000000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_ROCKCHIP_SERIAL=y
+CONFIG_ROCKCHIP_SPI=y
+CONFIG_SYSRESET=y
+CONFIG_USB=y
+CONFIG_ROCKCHIP_USB2_PHY=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Rockchip"
+CONFIG_USB_GADGET_VENDOR_NUM=0x2207
+CONFIG_USB_GADGET_PRODUCT_NUM=0x320a
+CONFIG_USB_GADGET_DWC2_OTG=y
+CONFIG_USB_FUNCTION_MASS_STORAGE=y
+CONFIG_DM_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+CONFIG_USE_TINY_PRINTF=y
+CONFIG_CMD_DHRYSTONE=y
+CONFIG_ERRNO_STR=y
+# CONFIG_SPL_OF_LIBFDT is not set

Signature · 1 line

2.17.0

Quoted 5 lines

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +0 -0

index 76dbdc8715..a9c9f970a4 100644

arch/arm/mach-rockchip/sdram_common.c +23 -22

-	debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
-	for (ch = 0; ch < ch_num; ch++) {
-		rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
-			SYS_REG_RANK_MASK);
-		col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
-		bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
-		cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
-				SYS_REG_CS0_ROW_MASK);
-		cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
-				SYS_REG_CS1_ROW_MASK);
-		bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
-			SYS_REG_BW_MASK));
-		row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
-			SYS_REG_ROW_3_4_MASK;
+		u32 sys_reg = readl(reg);
+		u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
+			       & SYS_REG_NUM_CH_MASK);
 
-		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
+		for (ch = 0; ch < ch_num; ch++) {
+			rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
+				SYS_REG_RANK_MASK);
+			col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
+			bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
+			cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
+					SYS_REG_CS0_ROW_MASK);
+			cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
+					SYS_REG_CS1_ROW_MASK);
+			bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
+				SYS_REG_BW_MASK));
+			row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
+				SYS_REG_ROW_3_4_MASK;
 
-		if (rank > 1)
-			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
-		if (row_3_4)
-			chipsize_mb = chipsize_mb * 3 / 4;
-		size_mb += chipsize_mb;
-		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
-		      rank, col, bk, cs0_row, bw, row_3_4);
+			chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+
+			if (rank > 1)
+				chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
+			if (row_3_4)
+				chipsize_mb = chipsize_mb * 3 / 4;

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	return (size_t)size_mb << 20;

Signature · 1 line

2.17.0

CC Philipp and Simon due maintainership (you may want to use get_maintainer.pl in the future) and Kever as the original author of the file.

Quoted 12 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c line 14

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>

Is adding these headers really necessary?
common.h already includes kernel.h and sizes.h (with some redirections of config.h)

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

Is this really required? I don't see a way that size_mb will already be set at this point. But it blows up your diff that it takes quite a while to see that your only real change is the size_mb = min(...) part at the end.

Quoted 47 lines

arch/arm/mach-rockchip/sdram_common.c +23 -22

-	debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
-	for (ch = 0; ch < ch_num; ch++) {
-		rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
-			SYS_REG_RANK_MASK);
-		col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
-		bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
-		cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
-				SYS_REG_CS0_ROW_MASK);
-		cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
-				SYS_REG_CS1_ROW_MASK);
-		bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
-			SYS_REG_BW_MASK));
-		row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
-			SYS_REG_ROW_3_4_MASK;
+		u32 sys_reg = readl(reg);
+		u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
+			       & SYS_REG_NUM_CH_MASK);
 
-		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
+		for (ch = 0; ch < ch_num; ch++) {
+			rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
+				SYS_REG_RANK_MASK);
+			col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
+			bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
+			cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
+					SYS_REG_CS0_ROW_MASK);
+			cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
+					SYS_REG_CS1_ROW_MASK);
+			bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
+				SYS_REG_BW_MASK));
+			row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
+				SYS_REG_ROW_3_4_MASK;
 
-		if (rank > 1)
-			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
-		if (row_3_4)
-			chipsize_mb = chipsize_mb * 3 / 4;
-		size_mb += chipsize_mb;
-		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
-		      rank, col, bk, cs0_row, bw, row_3_4);
+			chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+
+			if (rank > 1)
+				chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
+			if (row_3_4)
+				chipsize_mb = chipsize_mb * 3 / 4;

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 60

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c +10 -0

+			size_mb += chipsize_mb;
+			debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
+			      rank, col, bk, cs0_row, bw, row_3_4);
+		}
+
+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it 
+		 */

That's not true for all Rockchip SoCs. On the RK3399 for example the upper limit is 0xf8000000. Even on the RK3288 DRAM address range is actually
0x00000000-0xfe00000

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 62

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +1 -0

+		size_mb = min(size_mb, 0xff000000/SZ_1M);
 	}

Should be 0xfe000000 (4G-32MB) for RK3288.
But there is already a define for that, SDRAM_MAX_SIZE is defined in rkxxx_common.h. Using that one would help to avoid possible breakage of other Rockchip SoCs.

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	return (size_t)size_mb << 20;

Signature · 1 line

2.17.0

Regards,
Klaus

Quoted 11 lines · klaus.goger at theobroma-systems.com

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 60

Quoted 5 lines · Marty E. Plummer

arch/arm/mach-rockchip/sdram_common.c +4 -0

+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it 
+		 */

That's not true for all Rockchip SoCs. On the RK3399 for example the upper limit is 0xf8000000. Even on the RK3288 DRAM address range is actually
0x00000000-0xfe00000

Quoted 8 lines

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +1 -0

+		size_mb = min(size_mb, 0xff000000/SZ_1M);

	}

Should be 0xfe000000 (4G-32MB) for RK3288.
But there is already a define for that, SDRAM_MAX_SIZE is defined in rkxxx_common.h. Using that one would help to avoid possible breakage of other Rockchip SoCs.

Oh, does that get #defined properly for each SoC? if so, I'll sub that in for better compat.

Quoted 19 lines · Marty E. Plummer

Quoted 19 lines · klaus.goger at theobroma-systems.com

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 60

Quoted 5 lines · Marty E. Plummer

arch/arm/mach-rockchip/sdram_common.c +4 -0

+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it 
+		 */

That's not true for all Rockchip SoCs. On the RK3399 for example the upper limit is 0xf8000000. Even on the RK3288 DRAM address range is actually 0x00000000-0xfe00000

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +1 -0

+		size_mb = min(size_mb, 0xff000000/SZ_1M);

	}

Should be 0xfe000000 (4G-32MB) for RK3288.
But there is already a define for that, SDRAM_MAX_SIZE is defined in rkxxx_common.h. Using that one would help to avoid possible breakage of other Rockchip SoCs.

Quoted 2 lines

Oh, does that get #defined properly for each SoC? if so, I'll sub that in for better compat.

In fact, could you or someone else help me to understand the #include chain here? I'm not certain as to what that is available in each header mentioned can be used in this file to get max compat across the rockchip lineup, and I don't want to break someone elses board/etc with something that 'works for me’.

Quoted 23 lines · Marty E. Plummer

Quoted 19 lines · Marty E. Plummer

Quoted 19 lines · klaus.goger at theobroma-systems.com

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 60

Quoted 5 lines · Marty E. Plummer

arch/arm/mach-rockchip/sdram_common.c +4 -0

+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it 
+		 */

That's not true for all Rockchip SoCs. On the RK3399 for example the upper limit is 0xf8000000. Even on the RK3288 DRAM address range is actually 0x00000000-0xfe00000

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +1 -0

+		size_mb = min(size_mb, 0xff000000/SZ_1M);

	}

Should be 0xfe000000 (4G-32MB) for RK3288.
But there is already a define for that, SDRAM_MAX_SIZE is defined in rkxxx_common.h. Using that one would help to avoid possible breakage of other Rockchip SoCs.

Quoted 7 lines

Quoted 2 lines

Oh, does that get #defined properly for each SoC? if so, I'll sub that in for better compat.

In fact, could you or someone else help me to understand the #include chain here? I'm not certain as to what that is available in each header mentioned can be used in this file to get max compat across the rockchip lineup, and I don't want to break someone elses board/etc with something that 'works for me'.

You can get a full list of included headers by adding the -H file to the gcc command line. make V=1 will help you to see how it's called on your system.

In that particular case the relevant include chain is:
. include/common.h
.. include/config.h
... include/configs/popmetal_rk3288.h
.... include/configs/rk3288_common.h

So the correct rk3xxx_common.h will be selected by config.h and SDRAM_MAX_SIZE is defined in all of them.

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

arch/arm/mach-rockchip/sdram_common.c +34 -22

-	debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
-	for (ch = 0; ch < ch_num; ch++) {
-		rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
-			SYS_REG_RANK_MASK);
-		col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
-		bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
-		cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
-				SYS_REG_CS0_ROW_MASK);
-		cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
-				SYS_REG_CS1_ROW_MASK);
-		bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
-			SYS_REG_BW_MASK));
-		row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
-			SYS_REG_ROW_3_4_MASK;
+		u32 sys_reg = readl(reg);
+		u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
+			       & SYS_REG_NUM_CH_MASK);
 
-		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
+		for (ch = 0; ch < ch_num; ch++) {
+			rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
+				SYS_REG_RANK_MASK);
+			col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
+			bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
+			cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
+					SYS_REG_CS0_ROW_MASK);
+			cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
+					SYS_REG_CS1_ROW_MASK);
+			bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
+				SYS_REG_BW_MASK));
+			row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
+				SYS_REG_ROW_3_4_MASK;
 
-		if (rank > 1)
-			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
-		if (row_3_4)
-			chipsize_mb = chipsize_mb * 3 / 4;
-		size_mb += chipsize_mb;
-		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
-		      rank, col, bk, cs0_row, bw, row_3_4);
+			chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+
+			if (rank > 1)
+				chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
+			if (row_3_4)
+				chipsize_mb = chipsize_mb * 3 / 4;
+			size_mb += chipsize_mb;
+			debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
+			      rank, col, bk, cs0_row, bw, row_3_4);
+		}
+
+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);
 	}
 
 	return (size_t)size_mb << 20;

Signature · 6 lines

2.17.0

_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Quoted 5 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Quoted 2 lines

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

Could you elaborate what the change is and what root-cause this addresses (4GB reporting as 0 sounds a bit like a 32bit type overflowing)?
It's really hard to tell from the patch below (which seems to have everything simply reformatted to a different indentation)...

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

Given that there's a "size_mb = 0" just above it, this will always evaluate to true...

Quoted 61 lines

arch/arm/mach-rockchip/sdram_common.c +34 -22

-	debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
-	for (ch = 0; ch < ch_num; ch++) {
-		rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
-			SYS_REG_RANK_MASK);
-		col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
-		bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
-		cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
-				SYS_REG_CS0_ROW_MASK);
-		cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
-				SYS_REG_CS1_ROW_MASK);
-		bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
-			SYS_REG_BW_MASK));
-		row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
-			SYS_REG_ROW_3_4_MASK;
+		u32 sys_reg = readl(reg);
+		u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
+			       & SYS_REG_NUM_CH_MASK);
 
-		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
+		for (ch = 0; ch < ch_num; ch++) {
+			rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
+				SYS_REG_RANK_MASK);
+			col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
+			bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
+			cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
+					SYS_REG_CS0_ROW_MASK);
+			cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
+					SYS_REG_CS1_ROW_MASK);
+			bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
+				SYS_REG_BW_MASK));
+			row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
+				SYS_REG_ROW_3_4_MASK;
 
-		if (rank > 1)
-			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
-		if (row_3_4)
-			chipsize_mb = chipsize_mb * 3 / 4;
-		size_mb += chipsize_mb;
-		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
-		      rank, col, bk, cs0_row, bw, row_3_4);
+			chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+
+			if (rank > 1)
+				chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
+			if (row_3_4)
+				chipsize_mb = chipsize_mb * 3 / 4;
+			size_mb += chipsize_mb;
+			debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
+			      rank, col, bk, cs0_row, bw, row_3_4);
+		}
+
+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);
 	}
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	return (size_t)size_mb << 20;

Signature · 6 lines

2.17.0

_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

Signed-off-by Marty E. Plummer <[email protected]>

---
 arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------
 1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

arch/arm/mach-rockchip/sdram_common.c +18 -22

-	debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
-	for (ch = 0; ch < ch_num; ch++) {
-		rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
-			SYS_REG_RANK_MASK);
-		col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
-		bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
-		cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
-				SYS_REG_CS0_ROW_MASK);
-		cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
-				SYS_REG_CS1_ROW_MASK);
-		bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
-			SYS_REG_BW_MASK));
-		row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
-			SYS_REG_ROW_3_4_MASK;
+		u32 sys_reg = readl(reg);
+		u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
+			       & SYS_REG_NUM_CH_MASK);
 
-		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s %x %x\n", __func__, (u32)reg, sys_reg);
+		for (ch = 0; ch < ch_num; ch++) {
+			rank = 1 + (sys_reg >> SYS_REG_RANK_SHIFT(ch) &
+				SYS_REG_RANK_MASK);
+			col = 9 + (sys_reg >> SYS_REG_COL_SHIFT(ch) & SYS_REG_COL_MASK);
+			bk = 3 - ((sys_reg >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
+			cs0_row = 13 + (sys_reg >> SYS_REG_CS0_ROW_SHIFT(ch) &
+					SYS_REG_CS0_ROW_MASK);
+			cs1_row = 13 + (sys_reg >> SYS_REG_CS1_ROW_SHIFT(ch) &
+					SYS_REG_CS1_ROW_MASK);
+			bw = (2 >> ((sys_reg >> SYS_REG_BW_SHIFT(ch)) &
+				SYS_REG_BW_MASK));
+			row_3_4 = sys_reg >> SYS_REG_ROW_3_4_SHIFT(ch) &
+				SYS_REG_ROW_3_4_MASK;
 
-		if (rank > 1)
-			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
-		if (row_3_4)
-			chipsize_mb = chipsize_mb * 3 / 4;
-		size_mb += chipsize_mb;
-		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
-		      rank, col, bk, cs0_row, bw, row_3_4);
+			chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
 arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------
 1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 61

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Thanks,
- Kever

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 
 	return (size_t)size_mb << 20;

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 61

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 5 lines

Thanks,
- Kever

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 64

Quoted 1 line

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

Quoted 12 lines

Quoted 8 lines

Thanks,
- Kever

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 
 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

Quoted 64 lines · Dr. Philipp Tomsich

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 7 lines

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it:

arch/arm/mach-rockchip/sdram_common.c +4 -0

index 232a7fa655..0fe69bf558 100644
@@ -4,6 +4,7 @@
 * SPDX-License-Identifier:     GPL-2.0+
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <dm.h>
 #include <ram.h>

@@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_ROW_3_4_MASK;
 
 		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb);
 
 		if (rank > 1)
 			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
 		if (row_3_4)
 			chipsize_mb = chipsize_mb * 3 / 4;
 		size_mb += chipsize_mb;
+		debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
 		      rank, col, bk, cs0_row, bw, row_3_4);
 	}
 
+	debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 	size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M);
 
 	return (size_t)size_mb << 20;

drivers/ram/rockchip/sdram_rk3288.c +1 -1

index d99bf12476..9738eb088f 100644
@@ -7,6 +7,7 @@
 * Adapted from coreboot.
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
 
---

U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI

U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500)

Model: Google Speedy
DRAM: rockchip_sdram_size ff73009c 3c50dc50
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 800
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 1000
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 54: size_mb 1000
SDRAM base=0, size=fe000000
4 GiB

MMC:   dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0
In:    cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: Google Speedy
rockchip_dnl_key_pressed: adc_channel_single_shot fail!
Net:   Net Initialization Skipped

No ethernet found.
Hit any key to stop autoboot: 0
I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40

bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed.

Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container?

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Thanks,
- Kever

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 58

Quoted 1 line

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

Marty,

Quoted 146 lines · Marty E. Plummer

Quoted 64 lines · Dr. Philipp Tomsich

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 7 lines

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it:

arch/arm/mach-rockchip/sdram_common.c +4 -0

index 232a7fa655..0fe69bf558 100644
@@ -4,6 +4,7 @@
 * SPDX-License-Identifier:     GPL-2.0+
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <dm.h>
 #include <ram.h>

@@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_ROW_3_4_MASK;
 
 		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb);
 
 		if (rank > 1)
 			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
 		if (row_3_4)
 			chipsize_mb = chipsize_mb * 3 / 4;
 		size_mb += chipsize_mb;
+		debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
 		      rank, col, bk, cs0_row, bw, row_3_4);
 	}
 
+	debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 	size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M);
 
 	return (size_t)size_mb << 20;

drivers/ram/rockchip/sdram_rk3288.c +1 -1

index d99bf12476..9738eb088f 100644
@@ -7,6 +7,7 @@
 * Adapted from coreboot.
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
 
---

U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI

U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500)

Model: Google Speedy
DRAM: rockchip_sdram_size ff73009c 3c50dc50
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 800
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 1000
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 54: size_mb 1000
SDRAM base=0, size=fe000000
4 GiB

MMC:   dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0
In:    cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: Google Speedy
rockchip_dnl_key_pressed: adc_channel_single_shot fail!
Net:   Net Initialization Skipped

Quoted 4 lines

No ethernet found.
Hit any key to stop autoboot: 0
I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40

4GB is actually the 33rd bit set and the lower 32bits cleared (i.e. the largest 32bit value “plus one”).

Quoted 6 lines

bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed.

Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container?

The min() doesn’t make any sense here, as we implement the hook function ‘board_get_usable_ram_top’ just a few lines later…
We are at the start of the merge window right now, so I’d rather hold off a week (or two) and have a permanent solution than merging just a band-aid now and then having the full fix come in later during the merge window.

I briefly reviewed the situation yesterday and it looks like the size field in ram_info is the culprit: it’s defined as ‘size_t’, which again is __SIZE_TYPE__ which again is ‘unsigned int’ on a (32bit) arm-*-eabi compiler.

Expanding this to a phys_size_t won’t be doing us much good, either (as that one will also be 32bits for the RK3288).

The root cause of this is really that the RAM size and the ‘usable RAM’ are two different concepts in U-Boot. On a 32bit physical address space with memory-mapped peripherals, we can never have the full 4GB of DRAM as we’ll also have some of the physical address-space set aside for the MMIO; however, the MMIO range is only removed from the DRAM size when the usable ram-top is evaluated… so the size can be 4GB after all and overflow the 32bit size_t. Note that this separation into two different steps makes a lot of sense, as processors might not use MMIO but specialised instructions to access peripheral space—in which case there might indeed be a usable memory of 4GB on a 32bit physical address space.

From what I can tell, we’ll need to do two things:
(a) fix arch/arm/mach-rockchip/sdram_common.c to not use 32bit types
for the memory size
(b) touch ram.h to change the type of the ‘size’ field in ram_info (it needs
to be larger than 32bits

I’d like Simon’s input (as he owns ram.h) and can create a patchset for this change, if he agrees that this is the way forward.

Regards,
Philipp.

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 8 lines

Thanks,
- Kever

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 
 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot> <https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>>

Marty,

Quoted 146 lines · Marty E. Plummer

Quoted 64 lines · Dr. Philipp Tomsich

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 7 lines

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it:

arch/arm/mach-rockchip/sdram_common.c +4 -0

index 232a7fa655..0fe69bf558 100644
@@ -4,6 +4,7 @@
 * SPDX-License-Identifier:     GPL-2.0+
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <dm.h>
 #include <ram.h>

@@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_ROW_3_4_MASK;
 
 		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb);
 
 		if (rank > 1)
 			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
 		if (row_3_4)
 			chipsize_mb = chipsize_mb * 3 / 4;
 		size_mb += chipsize_mb;
+		debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
 		      rank, col, bk, cs0_row, bw, row_3_4);
 	}
 
+	debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 	size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M);
 
 	return (size_t)size_mb << 20;

drivers/ram/rockchip/sdram_rk3288.c +1 -1

index d99bf12476..9738eb088f 100644
@@ -7,6 +7,7 @@
 * Adapted from coreboot.
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
 
---

U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI

U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500)

Model: Google Speedy
DRAM: rockchip_sdram_size ff73009c 3c50dc50
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 800
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 1000
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 54: size_mb 1000
SDRAM base=0, size=fe000000
4 GiB

MMC:   dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0
In:    cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: Google Speedy
rockchip_dnl_key_pressed: adc_channel_single_shot fail!
Net:   Net Initialization Skipped

Quoted 4 lines

No ethernet found.
Hit any key to stop autoboot: 0
I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40

4GB is actually the 33rd bit set and the lower 32bits cleared (i.e. the largest 32bit value ???plus one???).

Quoted 6 lines

bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed.

Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container?

The min() doesn???t make any sense here, as we implement the hook function ???board_get_usable_ram_top??? just a few lines later???
We are at the start of the merge window right now, so I???d rather hold off a week (or two) and have a permanent solution than merging just a band-aid now and then having the full fix come in later during the merge window.

I briefly reviewed the situation yesterday and it looks like the size field in ram_info is the culprit: it???s defined as ???size_t???, which again is __SIZE_TYPE__ which again is ???unsigned int??? on a (32bit) arm-*-eabi compiler.

Expanding this to a phys_size_t won???t be doing us much good, either (as that one will also be 32bits for the RK3288).

The root cause of this is really that the RAM size and the ???usable RAM??? are two different concepts in U-Boot. On a 32bit physical address space with memory-mapped peripherals, we can never have the full 4GB of DRAM as we???ll also have some of the physical address-space set aside for the MMIO; however, the MMIO range is only removed from the DRAM size when the usable ram-top is evaluated??? so the size can be 4GB after all and overflow the 32bit size_t. Note that this separation into two different steps makes a lot of sense, as processors might not use MMIO but specialised instructions to access peripheral space???in which case there might indeed be a usable memory of 4GB on a 32bit physical address space.

From what I can tell, we???ll need to do two things:
(a) fix arch/arm/mach-rockchip/sdram_common.c to not use 32bit types
for the memory size
(b) touch ram.h to change the type of the ???size??? field in ram_info (it needs
to be larger than 32bits

I???d like Simon???s input (as he owns ram.h) and can create a patchset for this change, if he agrees that this is the way forward.

Regards,
Philipp.

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 8 lines

Thanks,
- Kever

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 
 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

Quoted 169 lines · Dr. Philipp Tomsich

Marty,

Quoted 146 lines · Marty E. Plummer

Quoted 64 lines · Dr. Philipp Tomsich

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 7 lines

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it:

arch/arm/mach-rockchip/sdram_common.c +4 -0

index 232a7fa655..0fe69bf558 100644
@@ -4,6 +4,7 @@
 * SPDX-License-Identifier:     GPL-2.0+
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <dm.h>
 #include <ram.h>

@@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_ROW_3_4_MASK;
 
 		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb);
 
 		if (rank > 1)
 			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
 		if (row_3_4)
 			chipsize_mb = chipsize_mb * 3 / 4;
 		size_mb += chipsize_mb;
+		debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
 		      rank, col, bk, cs0_row, bw, row_3_4);
 	}
 
+	debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 	size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M);
 
 	return (size_t)size_mb << 20;

drivers/ram/rockchip/sdram_rk3288.c +1 -1

index d99bf12476..9738eb088f 100644
@@ -7,6 +7,7 @@
 * Adapted from coreboot.
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
 
---

U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI

U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500)

Model: Google Speedy
DRAM: rockchip_sdram_size ff73009c 3c50dc50
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 800
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 1000
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 54: size_mb 1000
SDRAM base=0, size=fe000000
4 GiB

MMC:   dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0
In:    cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: Google Speedy
rockchip_dnl_key_pressed: adc_channel_single_shot fail!
Net:   Net Initialization Skipped

Quoted 4 lines

No ethernet found.
Hit any key to stop autoboot: 0
I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40

4GB is actually the 33rd bit set and the lower 32bits cleared (i.e. the largest 32bit value ???plus one???).

Quoted 6 lines

bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed.

Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container?

The min() doesn???t make any sense here, as we implement the hook function ???board_get_usable_ram_top??? just a few lines later???
We are at the start of the merge window right now, so I???d rather hold off a week (or two) and have a permanent solution than merging just a band-aid now and then having the full fix come in later during the merge window.

Quoted 3 lines

I briefly reviewed the situation yesterday and it looks like the size field in ram_info is the culprit: it???s defined as ???size_t???, which again is __SIZE_TYPE__ which again is ???unsigned int??? on a (32bit) arm-*-eabi compiler.

Yeah, I was talking about this with Marek on irc yesterday, I had the same conclusion. However, being very new and inexperienced with u-boot development in general, I'm a bit averse to to messing with what appears to be a global header file, as it can easily break any other board which uses it.

Quoted 32 lines

Expanding this to a phys_size_t won???t be doing us much good, either (as that one will also be 32bits for the RK3288).

The root cause of this is really that the RAM size and the ???usable RAM??? are two different concepts in U-Boot. On a 32bit physical address space with memory-mapped peripherals, we can never have the full 4GB of DRAM as we???ll also have some of the physical address-space set aside for the MMIO; however, the MMIO range is only removed from the DRAM size when the usable ram-top is evaluated??? so the size can be 4GB after all and overflow the 32bit size_t. Note that this separation into two different steps makes a lot of sense, as processors might not use MMIO but specialised instructions to access peripheral space???in which case there might indeed be a usable memory of 4GB on a 32bit physical address space.

From what I can tell, we???ll need to do two things:
(a) fix arch/arm/mach-rockchip/sdram_common.c to not use 32bit types
for the memory size
(b) touch ram.h to change the type of the ???size??? field in ram_info (it needs
to be larger than 32bits

I???d like Simon???s input (as he owns ram.h) and can create a patchset for this change, if he agrees that this is the way forward.

Regards,
Philipp.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Thanks,
- Kever

Quoted 2 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 58

Quoted 1 line

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

Quoted 173 lines · Marty E. Plummer

Quoted 169 lines · Dr. Philipp Tomsich

Marty,

Quoted 146 lines · Marty E. Plummer

Quoted 64 lines · Dr. Philipp Tomsich

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 7 lines

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it:

arch/arm/mach-rockchip/sdram_common.c +4 -0

index 232a7fa655..0fe69bf558 100644
@@ -4,6 +4,7 @@
 * SPDX-License-Identifier:     GPL-2.0+
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <dm.h>
 #include <ram.h>

@@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_ROW_3_4_MASK;
 
 		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb);
 
 		if (rank > 1)
 			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
 		if (row_3_4)
 			chipsize_mb = chipsize_mb * 3 / 4;
 		size_mb += chipsize_mb;
+		debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
 		      rank, col, bk, cs0_row, bw, row_3_4);
 	}
 
+	debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 	size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M);
 
 	return (size_t)size_mb << 20;

drivers/ram/rockchip/sdram_rk3288.c +1 -1

index d99bf12476..9738eb088f 100644
@@ -7,6 +7,7 @@
 * Adapted from coreboot.
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
 
---

U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI

U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500)

Model: Google Speedy
DRAM: rockchip_sdram_size ff73009c 3c50dc50
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 800
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 1000
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 54: size_mb 1000
SDRAM base=0, size=fe000000
4 GiB

MMC:   dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0
In:    cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: Google Speedy
rockchip_dnl_key_pressed: adc_channel_single_shot fail!
Net:   Net Initialization Skipped

Quoted 4 lines

No ethernet found.
Hit any key to stop autoboot: 0
I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40

4GB is actually the 33rd bit set and the lower 32bits cleared (i.e. the largest 32bit value ???plus one???).

Quoted 6 lines

bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed.

Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container?

The min() doesn???t make any sense here, as we implement the hook function ???board_get_usable_ram_top??? just a few lines later???
We are at the start of the merge window right now, so I???d rather hold off a week (or two) and have a permanent solution than merging just a band-aid now and then having the full fix come in later during the merge window.

Quoted 8 lines

Quoted 3 lines

I briefly reviewed the situation yesterday and it looks like the size field in ram_info is the culprit: it???s defined as ???size_t???, which again is __SIZE_TYPE__ which again is ???unsigned int??? on a (32bit) arm-*-eabi compiler.

Yeah, I was talking about this with Marek on irc yesterday, I had the same conclusion. However, being very new and inexperienced with u-boot development in general, I'm a bit averse to to messing with what appears to be a global header file, as it can easily break any other board which uses it.

No problem ??? let???s wait for Simon???s input and I???ll create the patchset. I???ll need your help testing, as I don???t have any 32bit platforms w/ 4GB??? and I???ll let you know once something is ready for you to test.

Quoted 39 lines

Quoted 39 lines

Expanding this to a phys_size_t won???t be doing us much good, either (as that one will also be 32bits for the RK3288).

The root cause of this is really that the RAM size and the ???usable RAM??? are two different concepts in U-Boot. On a 32bit physical address space with memory-mapped peripherals, we can never have the full 4GB of DRAM as we???ll also have some of the physical address-space set aside for the MMIO; however, the MMIO range is only removed from the DRAM size when the usable ram-top is evaluated??? so the size can be 4GB after all and overflow the 32bit size_t. Note that this separation into two different steps makes a lot of sense, as processors might not use MMIO but specialised instructions to access peripheral space???in which case there might indeed be a usable memory of 4GB on a 32bit physical address space.

From what I can tell, we???ll need to do two things:
(a) fix arch/arm/mach-rockchip/sdram_common.c to not use 32bit types
for the memory size
(b) touch ram.h to change the type of the ???size??? field in ram_info (it needs
to be larger than 32bits

I???d like Simon???s input (as he owns ram.h) and can create a patchset for this change, if he agrees that this is the way forward.

Regards,
Philipp.

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 8 lines

Thanks,
- Kever

Quoted 3 lines

arch/arm/mach-rockchip/sdram_common.c +0 -0

 	}
 
 	return (size_t)size_mb << 20;

_______________________________________________
U-Boot mailing list
[email protected] <mailto:[email protected]>
https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

Quoted 182 lines · Dr. Philipp Tomsich

Quoted 181 lines · Marty E. Plummer

Quoted 169 lines · Dr. Philipp Tomsich

Marty,

Quoted 146 lines · Marty E. Plummer

Quoted 64 lines · Dr. Philipp Tomsich

Quoted 61 lines · Marty E. Plummer

Quoted 36 lines · Kever Yang

Hi Marty,

Quoted 4 lines · Marty E. Plummer

Taken from coreboot's src/soc/rockchip/rk3288/sdram.c

Without this change, my u-boot build for the asus c201 chromebook (4GiB) is incorrectly detected as 0 Bytes of ram.

I know the root cause for this issue, and I have a local patch for it. The rk3288 is 32bit, and 4GB size is just out of range, so we need to before the max size before return with '<<20'. Sorry for forgot to send it out.

Quoted 23 lines

Signed-off-by Marty E. Plummer <[email protected]>

---
arch/arm/mach-rockchip/sdram_common.c | 62 ++++++++++++++++-----------

1 file changed, 37 insertions(+), 25 deletions(-)

arch/arm/mach-rockchip/sdram_common.c +2 -0

index 76dbdc8715..a9c9f970a4 100644
@@ -10,6 +10,8 @@
 #include <asm/io.h>
 #include <asm/arch/sdram_common.h>
 #include <dm/uclass-internal.h>
+#include <linux/kernel.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 size_t rockchip_sdram_size(phys_addr_t reg)

@@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	size_t size_mb = 0;
 	u32 ch;
 

Quoted 8 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 24

Quoted 4 lines

arch/arm/mach-rockchip/sdram_common.c +1 -3

-	u32 sys_reg = readl(reg);
-	u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
-		       & SYS_REG_NUM_CH_MASK);
+	if (!size_mb) {

I don't understand this and follow up changes, we don't really need it, isn't it?
I think don't need the changes before here.

Yeah, that was just another level of indentation for the if (!size_mb) guard, but I've reworked the patch to not do that as it was pointed out that since size_mb is initialized to 0 prior.

Quoted 10 lines

arch/arm/mach-rockchip/sdram_common.c size_t rockchip_sdram_size(phys_addr_t reg) · line 30

Quoted 6 lines

arch/arm/mach-rockchip/sdram_common.c +6 -0

+		/*
+		 * we use the 0x00000000~0xfeffffff space
+		 * since 0xff000000~0xffffffff is soc register space
+		 * so we reserve it
+		 */
+		size_mb = min(size_mb, 0xff000000/SZ_1M);

This is what we really need, as Klaus point out, we need to use SDRAM_MAX_SIZE
instead of hard code.

Quoted 2 lines

Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed, build and boot tested on my hardware.

Quoted 7 lines

In that case you just masked the problem but didn???t solve it: assuming size_mb is size_t (I???ll assume this is 64bit, but did not check), then your 4GB is 0x1_0000_0000 ) which overflows to 0x0 when converted to a u32.

In other words: we need to figure out where the truncation occurs (image what happens if a new 32bit processor with LPAE comes out???).

A very valid point. With the following patch to sdram_common.c and sdram_rk3288.c applied I get the debug output that follows it:

arch/arm/mach-rockchip/sdram_common.c +4 -0

index 232a7fa655..0fe69bf558 100644
@@ -4,6 +4,7 @@
 * SPDX-License-Identifier:     GPL-2.0+
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <dm.h>
 #include <ram.h>

@@ -39,16 +40,19 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_ROW_3_4_MASK;
 
 		chipsize_mb = (1 << (cs0_row + col + bk + bw - 20));
+		debug("%s: %d: chipsize_mb %x\n", __func__, __LINE__, chipsize_mb);
 
 		if (rank > 1)
 			chipsize_mb += chipsize_mb >> (cs0_row - cs1_row);
 		if (row_3_4)
 			chipsize_mb = chipsize_mb * 3 / 4;
 		size_mb += chipsize_mb;
+		debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 		debug("rank %d col %d bk %d cs0_row %d bw %d row_3_4 %d\n",
 		      rank, col, bk, cs0_row, bw, row_3_4);
 	}
 
+	debug("%s: %d: size_mb %x\n", __func__, __LINE__, size_mb);
 	size_mb = min(size_mb, SDRAM_MAX_SIZE/SZ_1M);
 
 	return (size_t)size_mb << 20;

drivers/ram/rockchip/sdram_rk3288.c +1 -1

index d99bf12476..9738eb088f 100644
@@ -7,6 +7,7 @@
 * Adapted from coreboot.
 */
 
+#define DEBUG 1
 #include <common.h>
 #include <clk.h>
 #include <dm.h>
 
---

U-Boot SPL 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500) Trying to boot from SPI

U-Boot 2018.05-rc3-02370-g309384e84b-dirty (May 07 2018 - 19:42:15 -0500)

Model: Google Speedy
DRAM: rockchip_sdram_size ff73009c 3c50dc50
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 800
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 42: chipsize_mb 400
rockchip_sdram_size: 49: size_mb 1000
rank 2 col 11 bk 3 cs0_row 14 bw 2 row_3_4 0
rockchip_sdram_size: 54: size_mb 1000
SDRAM base=0, size=fe000000
4 GiB

MMC:   dwmmc@ff0c0000: 1, dwmmc@ff0d0000: 2, dwmmc@ff0f0000: 0
In:    cros-ec-keyb
Out:   vidconsole
Err:   vidconsole
Model: Google Speedy
rockchip_dnl_key_pressed: adc_channel_single_shot fail!
Net:   Net Initialization Skipped

Quoted 4 lines

No ethernet found.
Hit any key to stop autoboot: 0
I guess we need to change the size_t to something larger; unless I'm mistaken, that's a 32 bit value, right? and 0x100000000 is at least 40

4GB is actually the 33rd bit set and the lower 32bits cleared (i.e. the largest 32bit value ???plus one???).

Quoted 6 lines

bits, unless I'm missing the issue here somewhere. However, that would take a change to include/ram.h, and would impact far more than just rk3288/rockchip devices across the board, so I'm unsure how to proceed.

Use the min macro here for now, and begin work migrating the ram_info size member to a 64-bit container?

The min() doesn???t make any sense here, as we implement the hook function ???board_get_usable_ram_top??? just a few lines later???
We are at the start of the merge window right now, so I???d rather hold off a week (or two) and have a permanent solution than merging just a band-aid now and then having the full fix come in later during the merge window.

Quoted 3 lines

I briefly reviewed the situation yesterday and it looks like the size field in ram_info is the culprit: it???s defined as ???size_t???, which again is __SIZE_TYPE__ which again is ???unsigned int??? on a (32bit) arm-*-eabi compiler.

Yeah, I was talking about this with Marek on irc yesterday, I had the same conclusion. However, being very new and inexperienced with u-boot development in general, I'm a bit averse to to messing with what appears to be a global header file, as it can easily break any other board which uses it.