[PATCH v1 00/11] rockchip fixes and extend rk3568 support

[PATCH v1 00/11] rockchip fixes and extend rk3568 support

[Peter Geis]

to: Simon Glass <sjg@chromium.org>
to: Philipp Tomsich <philipp.tomsich@vrull.eu>
to: Kever Yang <kever.yang@rock-chips.com>
to: Lukasz Majewski <lukma@denx.de>
to: Sean Anderson <seanga2@gmail.com>
to: Peng Fan <peng.fan@nxp.com>
to: Jaehoon Chung <jh80.chung@samsung.com>
to: Heiko Stübner <heiko@sntech.de>
cc: u-boot@lists.denx.de

Good Evening,

The following is a few patches for rockchip mainline u-boot support.
Patches 1-3 are fixes for the rk3568 reset handler, rockchip emmc dma to
sram, and building the rockchip-sfc driver.
Patch 4 adds a sanity check for the minimum sfc frequency.
Patch 5 and 6 add adc support to spl and enable the rockchip recovery
handler in spl, before attempting to load u-boot.
Patch 7 enables rk3568 spl bootrom device detection.
Patch 8 enables automatic clock gating and other power saving features
on rk3568, which solves the chip running hotter compared to downstream.
Patch 9 and 10 move the dwc3 platform data to the chip specific code and
enable dwc3 otg support on rk3568.
Patch 11 is an RFC patch for fixing ram detection on rk3568. Downstream
goes about this a different way, where they implemented a special
library to handle this.

Please review and *especially test* patch 11.

Very Respectfully,
Peter Geis

Peter Geis (11):
  clk: rockchip: rk3568: fix reset handler
  mmc: sdhci: allow disabling sdma in spl
  spi: rockchip-sfc: fix building rockchip-sfc
  spi: rockchip-sfc: sanity check minimum freq
  spl: support adc drivers in spl
  rockchip: handle bootrom recovery mode in spl
  rockchip: rk3568: add boot device detection
  rockchip: rk3568: enable automatic clock gating
  rockchip: move dwc3 config to chip specific handler
  rockchip: rk3568: add dwc3 otg support
  [RFC] rockchip: rk356x: attempt to fix ram detection

 arch/arm/mach-rockchip/Kconfig         |   1 +
 arch/arm/mach-rockchip/Makefile        |   6 +-
 arch/arm/mach-rockchip/board.c         |  24 ------
 arch/arm/mach-rockchip/boot_mode.c     |   4 +-
 arch/arm/mach-rockchip/rk3399/rk3399.c |  29 +++++++
 arch/arm/mach-rockchip/rk3568/rk3568.c | 112 +++++++++++++++++++++++++
 arch/arm/mach-rockchip/sdram.c         |  19 +++--
 common/board_f.c                       |   7 ++
 common/spl/Kconfig                     |   5 ++
 drivers/Makefile                       |   1 +
 drivers/clk/rockchip/clk_rk3568.c      |   2 +
 drivers/mmc/Kconfig                    |   7 ++
 drivers/mmc/sdhci.c                    |   6 +-
 drivers/spi/rockchip_sfc.c             |   6 ++
 include/configs/rk3568_common.h        |   5 ++
 15 files changed, 197 insertions(+), 37 deletions(-)

-- 
2.25.1

[PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Peter Geis]

The reset handler for rk3568 is missing its private data. This leads to
an abort when a reset is triggered.

Add the missing dev_set_priv to the rk3568 clk driver.

Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 drivers/clk/rockchip/clk_rk3568.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
index d5e45e7602c7..c83ae22dc302 100644
--- a/drivers/clk/rockchip/clk_rk3568.c
+++ b/drivers/clk/rockchip/clk_rk3568.c
@@ -14,6 +14,7 @@
 #include <asm/arch-rockchip/clock.h>
 #include <asm/arch-rockchip/hardware.h>
 #include <asm/io.h>
+#include <dm/device-internal.h>
 #include <dm/lists.h>
 #include <dt-bindings/clock/rk3568-cru.h>
 
@@ -2934,6 +2935,7 @@ static int rk3568_clk_bind(struct udevice *dev)
 						    glb_srst_fst);
 		priv->glb_srst_snd_value = offsetof(struct rk3568_cru,
 						    glb_srsr_snd);
+		dev_set_priv(sys_child, priv);
 	}
 
 #if CONFIG_IS_ENABLED(RESET_ROCKCHIP)
-- 
2.25.1

[PATCH v1 02/11] mmc: sdhci: allow disabling sdma in spl

[Peter Geis]

Rockchip emmc devices have a similar issue to Rockchip dwmmc devices,
where performing dma to sram causes errors with suspend/resume.
Allow us to toggle sdma in spl for sdhci similar to adma support, so we
can ensure dma is not used when loading the sram code.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 drivers/mmc/Kconfig | 7 +++++++
 drivers/mmc/sdhci.c | 6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index f04cc44e1973..1e4342285ce7 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -468,6 +468,13 @@ config MMC_SDHCI_SDMA
 	  This enables support for the SDMA (Single Operation DMA) defined
 	  in the SD Host Controller Standard Specification Version 1.00 .
 
+config SPL_MMC_SDHCI_SDMA
+	bool "Support SDHCI SDMA in SPL"
+	depends on MMC_SDHCI
+	help
+	  This enables support for the SDMA (Single Operation DMA) defined
+	  in the SD Host Controller Standard Specification Version 1.00 in SPL.
+
 config MMC_SDHCI_ADMA
 	bool "Support SDHCI ADMA2"
 	depends on MMC_SDHCI
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 766e4a6b0c5e..6285e53d12a2 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -70,7 +70,7 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
 	}
 }
 
-#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
+#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
 static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 			      int *is_aligned, int trans_bytes)
 {
@@ -177,7 +177,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
 		}
 	} while (!(stat & SDHCI_INT_DATA_END));
 
-#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
+#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
 	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
 			 mmc_get_dma_dir(data));
 #endif
@@ -836,7 +836,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
 #endif
 	debug("%s, caps: 0x%x\n", __func__, caps);
 
-#ifdef CONFIG_MMC_SDHCI_SDMA
+#if CONFIG_IS_ENABLED(MMC_SDHCI_SDMA)
 	if ((caps & SDHCI_CAN_DO_SDMA)) {
 		host->flags |= USE_SDMA;
 	} else {
-- 
2.25.1

[PATCH v1 03/11] spi: rockchip-sfc: fix building rockchip-sfc

[Peter Geis]

The rockchip-sfc driver is missing an include to build correctly.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 drivers/spi/rockchip_sfc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
index e098addddcac..851a6482985b 100644
--- a/drivers/spi/rockchip_sfc.c
+++ b/drivers/spi/rockchip_sfc.c
@@ -12,6 +12,7 @@
 #include <bouncebuf.h>
 #include <clk.h>
 #include <dm.h>
+#include <dm/device_compat.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/iopoll.h>
-- 
2.25.1

[PATCH v1 04/11] spi: rockchip-sfc: sanity check minimum freq

[Peter Geis]

The rockchip-sfc driver sanity checks the maximum frequency, but not the
minimum frequency.
This causes the probe to fail when a frequency isn't defined, such as
with `sf probe 0`.
Clamp the minimum frequency to the rockchip default clock rate.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 drivers/spi/rockchip_sfc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
index 851a6482985b..d0d2dc70a417 100644
--- a/drivers/spi/rockchip_sfc.c
+++ b/drivers/spi/rockchip_sfc.c
@@ -164,6 +164,8 @@
 /* DMA is only enabled for large data transmission */
 #define SFC_DMA_TRANS_THRETHOLD		(0x40)
 
+#define SFC_MIN_SPEED		(24 * 1000 * 1000)
+
 /* Maximum clock values from datasheet suggest keeping clock value under
  * 150MHz. No minimum or average value is suggested.
  */
@@ -596,6 +598,9 @@ static int rockchip_sfc_set_speed(struct udevice *bus, uint speed)
 	if (speed > sfc->max_freq)
 		speed = sfc->max_freq;
 
+	if (speed < SFC_MIN_SPEED)
+		speed = SFC_MIN_SPEED;
+
 	if (speed == sfc->speed)
 		return 0;
 
-- 
2.25.1

[PATCH v1 05/11] spl: support adc drivers in spl

[Peter Geis]

In order to handle the rockchip recovery handler in spl, we need the adc
code to be available in spl.
Add a toggle to allow adc drivers to function in spl.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 common/spl/Kconfig | 5 +++++
 drivers/Makefile   | 1 +
 2 files changed, 6 insertions(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index e0d0a6f77b51..df99042e2fd6 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -454,6 +454,11 @@ config SPL_FIT_IMAGE_TINY
 	  ensure this information is available to the next image
 	  invoked).
 
+config SPL_ADC
+	bool "Support ADC drivers in SPL"
+	help
+	  Enable ADC drivers in SPL.
+
 config SPL_CACHE
 	bool "Support CACHE drivers"
 	help
diff --git a/drivers/Makefile b/drivers/Makefile
index 4e7cf284405a..ce091ca9a7a4 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+
 
+obj-$(CONFIG_$(SPL_)ADC) += adc/
 obj-$(CONFIG_$(SPL_TPL_)BOOTCOUNT_LIMIT) += bootcount/
 obj-$(CONFIG_$(SPL_TPL_)BUTTON) += button/
 obj-$(CONFIG_$(SPL_TPL_)CACHE) += cache/
-- 
2.25.1

[PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Peter Geis]

Fixup the bootrom recovery mode code to function in spl, so we can
handle recovery mode in case u-boot loading is broken.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 arch/arm/mach-rockchip/Makefile        |  6 +++---
 arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
 arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index 00aef0ecee6a..53aff25ce8f6 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
 
 obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
 
-ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
-
 # Always include boot_mode.o, as we bypass it (i.e. turn it off)
 # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
 # we can have the preprocessor correctly recognise both 0x0 and 0
 # meaning "turn it off".
-obj-y += boot_mode.o
+obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
+
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
 obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
 obj-$(CONFIG_MISC_INIT_R) += misc.o
 endif
diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
index 1a1a887fc2cd..43cb369465a2 100644
--- a/arch/arm/mach-rockchip/boot_mode.c
+++ b/arch/arm/mach-rockchip/boot_mode.c
@@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
 	ret = -ENODEV;
 	uclass_foreach_dev(dev, uc) {
 		if (!strncmp(dev->name, "saradc", 6)) {
-			ret = adc_channel_single_shot(dev->name, 1, &val);
+			ret = adc_channel_single_shot(dev->name, 0, &val);
 			break;
 		}
 	}
@@ -89,6 +89,7 @@ int setup_boot_mode(void)
 	boot_mode = readl(reg);
 	debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
 
+#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
 	/* Clear boot mode */
 	writel(BOOT_NORMAL, reg);
 
@@ -102,6 +103,7 @@ int setup_boot_mode(void)
 		env_set("preboot", "setenv preboot; ums mmc 0");
 		break;
 	}
+#endif
 
 	return 0;
 }
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
index 22eeb77d41fa..4e23feb9417f 100644
--- a/arch/arm/mach-rockchip/rk3568/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
@@ -104,3 +104,26 @@ int arch_cpu_init(void)
 #endif
 	return 0;
 }
+
+#ifdef CONFIG_SPL_BUILD
+
+void __weak led_setup(void)
+{
+}
+
+void spl_board_init(void)
+{
+	led_setup();
+
+#if defined(SPL_DM_REGULATOR)
+	/*
+	 * Turning the eMMC and SPI back on (if disabled via the Qseven
+	 * BIOS_ENABLE) signal is done through a always-on regulator).
+	 */
+	if (regulators_enable_boot_on(false))
+		debug("%s: Cannot enable boot on regulator\n", __func__);
+#endif
+
+	setup_boot_mode();
+}
+#endif
-- 
2.25.1

[PATCH v1 07/11] rockchip: rk3568: add boot device detection

[Peter Geis]

Enable rk3568 spl to detect which device it was booted from.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 arch/arm/mach-rockchip/rk3568/rk3568.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
index 4e23feb9417f..5f239d89a7a9 100644
--- a/arch/arm/mach-rockchip/rk3568/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
@@ -7,6 +7,7 @@
 #include <dm.h>
 #include <asm/armv8/mmu.h>
 #include <asm/io.h>
+#include <asm/arch-rockchip/bootrom.h>
 #include <asm/arch-rockchip/grf_rk3568.h>
 #include <asm/arch-rockchip/hardware.h>
 #include <dt-bindings/clock/rk3568-cru.h>
@@ -23,6 +24,7 @@
 #define SGRF_SOC_CON4			0x10
 #define EMMC_HPROT_SECURE_CTRL		0x03
 #define SDMMC0_HPROT_SECURE_CTRL	0x01
+
 /* PMU_GRF_GPIO0D_IOMUX_L */
 enum {
 	GPIO0D1_SHIFT		= 4,
@@ -43,6 +45,12 @@ enum {
 	UART2_IO_SEL_M0		= 0,
 };
 
+const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
+	[BROM_BOOTSOURCE_EMMC] = "/sdhci@fe310000",
+	[BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
+	[BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
+};
+
 static struct mm_region rk3568_mem_map[] = {
 	{
 		.virt = 0x0UL,
-- 
2.25.1

[PATCH v1 08/11] rockchip: rk3568: enable automatic clock gating

[Peter Geis]

Enable automatic clock gating on rk3568, which solves a 7c temperature
difference on SoQuartz compared to downstream.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
index 5f239d89a7a9..0e0a7f5b54f2 100644
--- a/arch/arm/mach-rockchip/rk3568/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
@@ -25,6 +25,15 @@
 #define EMMC_HPROT_SECURE_CTRL		0x03
 #define SDMMC0_HPROT_SECURE_CTRL	0x01
 
+#define PMU_BASE_ADDR		0xfdd90000
+#define PMU_NOC_AUTO_CON0	(0x70)
+#define PMU_NOC_AUTO_CON1	(0x74)
+#define EDP_PHY_GRF_BASE	0xfdcb0000
+#define EDP_PHY_GRF_CON0	(EDP_PHY_GRF_BASE + 0x00)
+#define EDP_PHY_GRF_CON10	(EDP_PHY_GRF_BASE + 0x28)
+#define CPU_GRF_BASE		0xfdc30000
+#define GRF_CORE_PVTPLL_CON0	(0x10)
+
 /* PMU_GRF_GPIO0D_IOMUX_L */
 enum {
 	GPIO0D1_SHIFT		= 4,
@@ -99,6 +108,20 @@ void board_debug_uart_init(void)
 int arch_cpu_init(void)
 {
 #ifdef CONFIG_SPL_BUILD
+	/*
+	 * When perform idle operation, corresponding clock can
+	 * be opened or gated automatically.
+	 */
+	writel(0xffffffff, PMU_BASE_ADDR + PMU_NOC_AUTO_CON0);
+	writel(0x000f000f, PMU_BASE_ADDR + PMU_NOC_AUTO_CON1);
+
+	/* Disable eDP phy by default */
+	writel(0x00070007, EDP_PHY_GRF_CON10);
+	writel(0x0ff10ff1, EDP_PHY_GRF_CON0);
+
+	/* Set core pvtpll ring length */
+	writel(0x00ff002b, CPU_GRF_BASE + GRF_CORE_PVTPLL_CON0);
+
 	/* Set the emmc sdmmc0 to secure */
 	rk_clrreg(SGRF_BASE + SGRF_SOC_CON4, (EMMC_HPROT_SECURE_CTRL << 11
 		| SDMMC0_HPROT_SECURE_CTRL << 4));
-- 
2.25.1

[PATCH v1 09/11] rockchip: move dwc3 config to chip specific handler

[Peter Geis]

The dwc3 code in the mach-rockchip board file is specific to the rk3399.
Move it to the rk3399 chip specific code.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 arch/arm/mach-rockchip/board.c         | 24 ---------------------
 arch/arm/mach-rockchip/rk3399/rk3399.c | 29 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 5304eb055c6d..b19c15c26f2e 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -127,30 +127,6 @@ int board_usb_cleanup(int index, enum usb_init_type init)
 }
 #endif /* CONFIG_USB_GADGET_DWC2_OTG */
 
-#if defined(CONFIG_USB_DWC3_GADGET) && !defined(CONFIG_DM_USB_GADGET)
-#include <dwc3-uboot.h>
-
-static struct dwc3_device dwc3_device_data = {
-	.maximum_speed = USB_SPEED_HIGH,
-	.base = 0xfe800000,
-	.dr_mode = USB_DR_MODE_PERIPHERAL,
-	.index = 0,
-	.dis_u2_susphy_quirk = 1,
-	.hsphy_mode = USBPHY_INTERFACE_MODE_UTMIW,
-};
-
-int usb_gadget_handle_interrupts(int index)
-{
-	dwc3_uboot_handle_interrupt(0);
-	return 0;
-}
-
-int board_usb_init(int index, enum usb_init_type init)
-{
-	return dwc3_uboot_init(&dwc3_device_data);
-}
-#endif /* CONFIG_USB_DWC3_GADGET */
-
 #endif /* CONFIG_USB_GADGET */
 
 #if CONFIG_IS_ENABLED(FASTBOOT)
diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
index d40969c88898..c7e38997e521 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -284,3 +284,32 @@ void spl_board_init(void)
 #endif
 }
 #endif
+
+#if defined(CONFIG_USB_GADGET)
+#include <usb.h>
+
+#if defined(CONFIG_USB_DWC3_GADGET) && !defined(CONFIG_DM_USB_GADGET)
+#include <dwc3-uboot.h>
+
+static struct dwc3_device dwc3_device_data = {
+	.maximum_speed = USB_SPEED_HIGH,
+	.base = 0xfe800000,
+	.dr_mode = USB_DR_MODE_PERIPHERAL,
+	.index = 0,
+	.dis_u2_susphy_quirk = 1,
+	.hsphy_mode = USBPHY_INTERFACE_MODE_UTMIW,
+};
+
+int usb_gadget_handle_interrupts(int index)
+{
+	dwc3_uboot_handle_interrupt(0);
+	return 0;
+}
+
+int board_usb_init(int index, enum usb_init_type init)
+{
+	return dwc3_uboot_init(&dwc3_device_data);
+}
+#endif /* CONFIG_USB_DWC3_GADGET */
+
+#endif /* CONFIG_USB_GADGET */
-- 
2.25.1

[PATCH v1 10/11] rockchip: rk3568: add dwc3 otg support

[Peter Geis]

Add the required platform data to the rk3568 chip config, in order to
support dwc3 otg on this chip.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
index 0e0a7f5b54f2..ef6bc67a88b0 100644
--- a/arch/arm/mach-rockchip/rk3568/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
@@ -158,3 +158,32 @@ void spl_board_init(void)
 	setup_boot_mode();
 }
 #endif
+
+#if defined(CONFIG_USB_GADGET)
+#include <usb.h>
+
+#if defined(CONFIG_USB_DWC3_GADGET) && !defined(CONFIG_DM_USB_GADGET)
+#include <dwc3-uboot.h>
+
+static struct dwc3_device dwc3_device_data = {
+	.maximum_speed = USB_SPEED_HIGH,
+	.base = 0xfcc00000,
+	.dr_mode = USB_DR_MODE_PERIPHERAL,
+	.index = 0,
+	.dis_u2_susphy_quirk = 1,
+	.hsphy_mode = USBPHY_INTERFACE_MODE_UTMIW,
+};
+
+int usb_gadget_handle_interrupts(int index)
+{
+	dwc3_uboot_handle_interrupt(0);
+	return 0;
+}
+
+int board_usb_init(int index, enum usb_init_type init)
+{
+	return dwc3_uboot_init(&dwc3_device_data);
+}
+#endif /* CONFIG_USB_DWC3_GADGET */
+
+#endif /* CONFIG_USB_GADGET */
-- 
2.25.1

[PATCH v1 11/11] [RFC] rockchip: rk356x: attempt to fix ram detection

[Peter Geis]

This patch attempts to fix ram detection on rk3568.
Prior to this, the rk3568 incorrectly detected 8gb ram as 2gb.
On top of this, the board panics when u-boot accesses ram above 4gb.

Fix this by correcting ram detection in hopefully a backwards compatable
way, and extend board_f.c to enforce an upper limit on the ram u-boot
uses.
This allows us to limit the ram u-boot accesses, while passing the
correctly detected size to follow on software (eg linux).

This has been tested on rk3566 2gb, 4gb, and 8gb configurations, as well
as rk3399 4gb configurations.

I do not have other configurations available, and I do not have the
insights into rockchip ram handling to tell if this is the correct way
to go about this.

Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
 arch/arm/mach-rockchip/Kconfig         |  1 +
 arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
 arch/arm/mach-rockchip/sdram.c         | 19 +++++++++++------
 common/board_f.c                       |  7 +++++++
 include/configs/rk3568_common.h        |  5 +++++
 5 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 92f35309e4a6..58393cc623f8 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -264,6 +264,7 @@ config ROCKCHIP_RK3568
 	select SYSCON
 	select BOARD_LATE_INIT
 	imply ROCKCHIP_COMMON_BOARD
+	imply OF_SYSTEM_SETUP
 	help
 	  The Rockchip RK3568 is a ARM-based SoC with quad-core Cortex-A55,
 	  including NEON and GPU, 512K L3 cache, Mali-G52 based graphics,
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
index ef6bc67a88b0..8d2a59bc649d 100644
--- a/arch/arm/mach-rockchip/rk3568/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
@@ -5,6 +5,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <fdt_support.h>
 #include <asm/armv8/mmu.h>
 #include <asm/io.h>
 #include <asm/arch-rockchip/bootrom.h>
@@ -187,3 +188,31 @@ int board_usb_init(int index, enum usb_init_type init)
 #endif /* CONFIG_USB_DWC3_GADGET */
 
 #endif /* CONFIG_USB_GADGET */
+
+#ifdef CONFIG_OF_SYSTEM_SETUP
+int ft_system_setup(void *blob, struct bd_info *bd)
+{
+	int ret;
+	int areas = 1;
+	u64 start[2], size[2];
+
+	/* Reserve the io address space. */
+	if (gd->ram_top > SDRAM_UPPER_ADDR_MIN) {
+		start[0] = gd->bd->bi_dram[0].start;
+		size[0] = SDRAM_LOWER_ADDR_MAX - gd->bd->bi_dram[0].start;
+
+		/* Add the upper 4GB address space */
+		start[1] = SDRAM_UPPER_ADDR_MIN;
+		size[1] = gd->ram_top - SDRAM_UPPER_ADDR_MIN;
+		areas = 2;
+
+		ret = fdt_set_usable_memory(blob, start, size, areas);
+		if (ret) {
+			printf("Cannot set usable memory\n");
+			return ret;
+		}
+	}
+
+	return 0;
+};
+#endif
diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
index 705ec7ba6450..52974e6dc333 100644
--- a/arch/arm/mach-rockchip/sdram.c
+++ b/arch/arm/mach-rockchip/sdram.c
@@ -3,6 +3,8 @@
  * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
  */
 
+#define DEBUG
+
 #include <common.h>
 #include <dm.h>
 #include <init.h>
@@ -98,8 +100,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			  SYS_REG_COL_MASK);
 		cs1_col = cs0_col;
 		bk = 3 - ((sys_reg2 >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
-		if ((sys_reg3 >> SYS_REG_VERSION_SHIFT &
-		     SYS_REG_VERSION_MASK) == 0x2) {
+		if ((sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) >= 0x2) {
 			cs1_col = 9 + (sys_reg3 >> SYS_REG_CS1_COL_SHIFT(ch) &
 				  SYS_REG_CS1_COL_MASK);
 			if (((sys_reg3 >> SYS_REG_EXTEND_CS0_ROW_SHIFT(ch) &
@@ -136,7 +137,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 			SYS_REG_BW_MASK));
 		row_3_4 = sys_reg2 >> SYS_REG_ROW_3_4_SHIFT(ch) &
 			SYS_REG_ROW_3_4_MASK;
-		if (dram_type == DDR4) {
+		if ((dram_type == DDR4) && (sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) != 0x3){
 			dbw = (sys_reg2 >> SYS_REG_DBW_SHIFT(ch)) &
 				SYS_REG_DBW_MASK;
 			bg = (dbw == 2) ? 2 : 1;
@@ -176,9 +177,11 @@ size_t rockchip_sdram_size(phys_addr_t reg)
 	 *   2. update board_get_usable_ram_top() and dram_init_banksize()
 	 *   to reserve memory for peripheral space after previous update.
 	 */
+
+#ifndef __aarch64__
 	if (size_mb > (SDRAM_MAX_SIZE >> 20))
 		size_mb = (SDRAM_MAX_SIZE >> 20);
-
+#endif
 	return (size_t)size_mb << 20;
 }
 
@@ -208,6 +211,10 @@ int dram_init(void)
 ulong board_get_usable_ram_top(ulong total_size)
 {
 	unsigned long top = CONFIG_SYS_SDRAM_BASE + SDRAM_MAX_SIZE;
-
-	return (gd->ram_top > top) ? top : gd->ram_top;
+#ifdef SDRAM_UPPER_ADDR_MIN
+	if (gd->ram_top > SDRAM_UPPER_ADDR_MIN)
+		return gd->ram_top;
+	else
+#endif
+		return (gd->ram_top > top) ? top : gd->ram_top;
 }
diff --git a/common/board_f.c b/common/board_f.c
index a68760092ac1..933ba7aedac0 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -344,7 +344,14 @@ static int setup_dest_addr(void)
 #endif
 	gd->ram_top = gd->ram_base + get_effective_memsize();
 	gd->ram_top = board_get_usable_ram_top(gd->mon_len);
+#ifdef SDRAM_LOWER_ADDR_MAX
+	if (gd->ram_top > SDRAM_LOWER_ADDR_MAX)
+		gd->relocaddr = SDRAM_LOWER_ADDR_MAX;
+	else
+		gd->relocaddr = gd->ram_top;
+#else
 	gd->relocaddr = gd->ram_top;
+#endif
 	debug("Ram top: %08lX\n", (ulong)gd->ram_top);
 #if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
 	/*
diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h
index 25d7c5cc8fff..8dd1b033017b 100644
--- a/include/configs/rk3568_common.h
+++ b/include/configs/rk3568_common.h
@@ -27,6 +27,11 @@
 #define CONFIG_SYS_SDRAM_BASE		0
 #define SDRAM_MAX_SIZE			0xf0000000
 
+#ifdef CONFIG_OF_SYSTEM_SETUP
+#define SDRAM_LOWER_ADDR_MAX		0xf0000000
+#define SDRAM_UPPER_ADDR_MIN		0x100000000
+#endif
+
 #ifndef CONFIG_SPL_BUILD
 #define ENV_MEM_LAYOUT_SETTINGS		\
 	"scriptaddr=0x00c00000\0"	\
-- 
2.25.1

Re: [PATCH v1 02/11] mmc: sdhci: allow disabling sdma in spl

[Jaehoon Chung]

On 2/22/22 10:31, Peter Geis wrote:
> Rockchip emmc devices have a similar issue to Rockchip dwmmc devices,
> where performing dma to sram causes errors with suspend/resume.
> Allow us to toggle sdma in spl for sdhci similar to adma support, so we
> can ensure dma is not used when loading the sram code.
> 
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/Kconfig | 7 +++++++
>  drivers/mmc/sdhci.c | 6 +++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index f04cc44e1973..1e4342285ce7 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -468,6 +468,13 @@ config MMC_SDHCI_SDMA
>  	  This enables support for the SDMA (Single Operation DMA) defined
>  	  in the SD Host Controller Standard Specification Version 1.00 .
>  
> +config SPL_MMC_SDHCI_SDMA
> +	bool "Support SDHCI SDMA in SPL"
> +	depends on MMC_SDHCI
> +	help
> +	  This enables support for the SDMA (Single Operation DMA) defined
> +	  in the SD Host Controller Standard Specification Version 1.00 in SPL.
> +
>  config MMC_SDHCI_ADMA
>  	bool "Support SDHCI ADMA2"
>  	depends on MMC_SDHCI
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 766e4a6b0c5e..6285e53d12a2 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -70,7 +70,7 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
>  	}
>  }
>  
> -#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
> +#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>  static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
>  			      int *is_aligned, int trans_bytes)
>  {
> @@ -177,7 +177,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
>  		}
>  	} while (!(stat & SDHCI_INT_DATA_END));
>  
> -#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
> +#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>  	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>  			 mmc_get_dma_dir(data));
>  #endif
> @@ -836,7 +836,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
>  #endif
>  	debug("%s, caps: 0x%x\n", __func__, caps);
>  
> -#ifdef CONFIG_MMC_SDHCI_SDMA
> +#if CONFIG_IS_ENABLED(MMC_SDHCI_SDMA)
>  	if ((caps & SDHCI_CAN_DO_SDMA)) {
>  		host->flags |= USE_SDMA;
>  	} else {

Re: [PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Simon Glass]

Hi Peter,

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> The reset handler for rk3568 is missing its private data. This leads to
> an abort when a reset is triggered.
>
> Add the missing dev_set_priv to the rk3568 clk driver.
>
> Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  drivers/clk/rockchip/clk_rk3568.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
> index d5e45e7602c7..c83ae22dc302 100644
> --- a/drivers/clk/rockchip/clk_rk3568.c
> +++ b/drivers/clk/rockchip/clk_rk3568.c
> @@ -14,6 +14,7 @@
>  #include <asm/arch-rockchip/clock.h>
>  #include <asm/arch-rockchip/hardware.h>
>  #include <asm/io.h>
> +#include <dm/device-internal.h>
>  #include <dm/lists.h>
>  #include <dt-bindings/clock/rk3568-cru.h>
>
> @@ -2934,6 +2935,7 @@ static int rk3568_clk_bind(struct udevice *dev)
>                                                     glb_srst_fst);
>                 priv->glb_srst_snd_value = offsetof(struct rk3568_cru,
>                                                     glb_srsr_snd);
> +               dev_set_priv(sys_child, priv);
>         }
>
>  #if CONFIG_IS_ENABLED(RESET_ROCKCHIP)
> --
> 2.25.1
>

You must not access priv data in the bind() handler as it doesn't
exist yet. The malloc() in that function is incorrect.

You certainly must not set the priv data in there. See the lifecycle
docs in driver model for how things are supposed to work.

You can use plat data, so could move glb_srst_fst_value and
glb_srst_snd_value to struct rk3568_clk_plat, although oddly that
struct only exists if OF_PLATDATA is used.

Quite confusing.

Regards,
Simon

Re: [PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Simon Glass]

Hi Peter,

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> Fixup the bootrom recovery mode code to function in spl, so we can
> handle recovery mode in case u-boot loading is broken.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/Makefile        |  6 +++---
>  arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
>  arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index 00aef0ecee6a..53aff25ce8f6 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
>
>  obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
>
> -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> -
>  # Always include boot_mode.o, as we bypass it (i.e. turn it off)
>  # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
>  # we can have the preprocessor correctly recognise both 0x0 and 0
>  # meaning "turn it off".
> -obj-y += boot_mode.o
> +obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
> +
> +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
>  obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
>  obj-$(CONFIG_MISC_INIT_R) += misc.o
>  endif
> diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
> index 1a1a887fc2cd..43cb369465a2 100644
> --- a/arch/arm/mach-rockchip/boot_mode.c
> +++ b/arch/arm/mach-rockchip/boot_mode.c
> @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
>         ret = -ENODEV;
>         uclass_foreach_dev(dev, uc) {
>                 if (!strncmp(dev->name, "saradc", 6)) {
> -                       ret = adc_channel_single_shot(dev->name, 1, &val);
> +                       ret = adc_channel_single_shot(dev->name, 0, &val);
>                         break;
>                 }
>         }
> @@ -89,6 +89,7 @@ int setup_boot_mode(void)
>         boot_mode = readl(reg);
>         debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
>
> +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)

Can you use if() ?

>         /* Clear boot mode */
>         writel(BOOT_NORMAL, reg);
>
> @@ -102,6 +103,7 @@ int setup_boot_mode(void)
>                 env_set("preboot", "setenv preboot; ums mmc 0");
>                 break;
>         }
> +#endif
>
>         return 0;
>  }
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 22eeb77d41fa..4e23feb9417f 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -104,3 +104,26 @@ int arch_cpu_init(void)
>  #endif
>         return 0;
>  }
> +
> +#ifdef CONFIG_SPL_BUILD

Do you need that?

> +
> +void __weak led_setup(void)
> +{
> +}
> +
> +void spl_board_init(void)
> +{
> +       led_setup();
> +
> +#if defined(SPL_DM_REGULATOR)

You need a CONFIG_ prefix there.

But better:

if (CONFIG_IS_ENABLED(DM_REGULATOR))

> +       /*
> +        * Turning the eMMC and SPI back on (if disabled via the Qseven
> +        * BIOS_ENABLE) signal is done through a always-on regulator).
> +        */
> +       if (regulators_enable_boot_on(false))
> +               debug("%s: Cannot enable boot on regulator\n", __func__);
> +#endif
> +
> +       setup_boot_mode();
> +}
> +#endif
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v1 07/11] rockchip: rk3568: add boot device detection

[Simon Glass]

Hi Peter,

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> Enable rk3568 spl to detect which device it was booted from.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/rk3568/rk3568.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

I think a better way would be to put an option in the U-Boot config node, like:

boot-devices = <&sdhci_0 &spi_flash &sd>;

> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 4e23feb9417f..5f239d89a7a9 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -7,6 +7,7 @@
>  #include <dm.h>
>  #include <asm/armv8/mmu.h>
>  #include <asm/io.h>
> +#include <asm/arch-rockchip/bootrom.h>
>  #include <asm/arch-rockchip/grf_rk3568.h>
>  #include <asm/arch-rockchip/hardware.h>
>  #include <dt-bindings/clock/rk3568-cru.h>
> @@ -23,6 +24,7 @@
>  #define SGRF_SOC_CON4                  0x10
>  #define EMMC_HPROT_SECURE_CTRL         0x03
>  #define SDMMC0_HPROT_SECURE_CTRL       0x01
> +
>  /* PMU_GRF_GPIO0D_IOMUX_L */
>  enum {
>         GPIO0D1_SHIFT           = 4,
> @@ -43,6 +45,12 @@ enum {
>         UART2_IO_SEL_M0         = 0,
>  };
>
> +const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> +       [BROM_BOOTSOURCE_EMMC] = "/sdhci@fe310000",
> +       [BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
> +       [BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
> +};
> +
>  static struct mm_region rk3568_mem_map[] = {
>         {
>                 .virt = 0x0UL,
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v1 08/11] rockchip: rk3568: enable automatic clock gating

[Simon Glass]

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> Enable automatic clock gating on rk3568, which solves a 7c temperature
> difference on SoQuartz compared to downstream.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v1 09/11] rockchip: move dwc3 config to chip specific handler

[Simon Glass]

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> The dwc3 code in the mach-rockchip board file is specific to the rk3399.
> Move it to the rk3399 chip specific code.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/board.c         | 24 ---------------------
>  arch/arm/mach-rockchip/rk3399/rk3399.c | 29 ++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v1 10/11] rockchip: rk3568: add dwc3 otg support

[Simon Glass]

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> Add the required platform data to the rk3568 chip config, in order to
> support dwc3 otg on this chip.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Seems that this info should be in the device tree.

Re: [PATCH v1 11/11] [RFC] rockchip: rk356x: attempt to fix ram detection

[Simon Glass]

Hi Peter,

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> This patch attempts to fix ram detection on rk3568.
> Prior to this, the rk3568 incorrectly detected 8gb ram as 2gb.
> On top of this, the board panics when u-boot accesses ram above 4gb.
>
> Fix this by correcting ram detection in hopefully a backwards compatable
> way, and extend board_f.c to enforce an upper limit on the ram u-boot
> uses.
> This allows us to limit the ram u-boot accesses, while passing the
> correctly detected size to follow on software (eg linux).
>
> This has been tested on rk3566 2gb, 4gb, and 8gb configurations, as well
> as rk3399 4gb configurations.
>
> I do not have other configurations available, and I do not have the
> insights into rockchip ram handling to tell if this is the correct way
> to go about this.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/Kconfig         |  1 +
>  arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
>  arch/arm/mach-rockchip/sdram.c         | 19 +++++++++++------
>  common/board_f.c                       |  7 +++++++
>  include/configs/rk3568_common.h        |  5 +++++
>  5 files changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index 92f35309e4a6..58393cc623f8 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -264,6 +264,7 @@ config ROCKCHIP_RK3568
>         select SYSCON
>         select BOARD_LATE_INIT
>         imply ROCKCHIP_COMMON_BOARD
> +       imply OF_SYSTEM_SETUP
>         help
>           The Rockchip RK3568 is a ARM-based SoC with quad-core Cortex-A55,
>           including NEON and GPU, 512K L3 cache, Mali-G52 based graphics,
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index ef6bc67a88b0..8d2a59bc649d 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -5,6 +5,7 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <fdt_support.h>
>  #include <asm/armv8/mmu.h>
>  #include <asm/io.h>
>  #include <asm/arch-rockchip/bootrom.h>
> @@ -187,3 +188,31 @@ int board_usb_init(int index, enum usb_init_type init)
>  #endif /* CONFIG_USB_DWC3_GADGET */
>
>  #endif /* CONFIG_USB_GADGET */
> +
> +#ifdef CONFIG_OF_SYSTEM_SETUP
> +int ft_system_setup(void *blob, struct bd_info *bd)
> +{
> +       int ret;
> +       int areas = 1;
> +       u64 start[2], size[2];
> +
> +       /* Reserve the io address space. */
> +       if (gd->ram_top > SDRAM_UPPER_ADDR_MIN) {

Can you put SDRAM_UPPER_ADDR_MIN and SDRAM_LOWER_ADDR_MAX into the
device tree, perhaps?


> +               start[0] = gd->bd->bi_dram[0].start;
> +               size[0] = SDRAM_LOWER_ADDR_MAX - gd->bd->bi_dram[0].start;
> +
> +               /* Add the upper 4GB address space */
> +               start[1] = SDRAM_UPPER_ADDR_MIN;
> +               size[1] = gd->ram_top - SDRAM_UPPER_ADDR_MIN;
> +               areas = 2;
> +
> +               ret = fdt_set_usable_memory(blob, start, size, areas);
> +               if (ret) {
> +                       printf("Cannot set usable memory\n");
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;
> +};
> +#endif
> diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
> index 705ec7ba6450..52974e6dc333 100644
> --- a/arch/arm/mach-rockchip/sdram.c
> +++ b/arch/arm/mach-rockchip/sdram.c
> @@ -3,6 +3,8 @@
>   * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>   */
>
> +#define DEBUG
> +
>  #include <common.h>
>  #include <dm.h>
>  #include <init.h>
> @@ -98,8 +100,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
>                           SYS_REG_COL_MASK);
>                 cs1_col = cs0_col;
>                 bk = 3 - ((sys_reg2 >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
> -               if ((sys_reg3 >> SYS_REG_VERSION_SHIFT &
> -                    SYS_REG_VERSION_MASK) == 0x2) {
> +               if ((sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) >= 0x2) {
>                         cs1_col = 9 + (sys_reg3 >> SYS_REG_CS1_COL_SHIFT(ch) &
>                                   SYS_REG_CS1_COL_MASK);
>                         if (((sys_reg3 >> SYS_REG_EXTEND_CS0_ROW_SHIFT(ch) &
> @@ -136,7 +137,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
>                         SYS_REG_BW_MASK));
>                 row_3_4 = sys_reg2 >> SYS_REG_ROW_3_4_SHIFT(ch) &
>                         SYS_REG_ROW_3_4_MASK;
> -               if (dram_type == DDR4) {
> +               if ((dram_type == DDR4) && (sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) != 0x3){

Space before {

Also can you please add a comment as to what this is doing?

>                         dbw = (sys_reg2 >> SYS_REG_DBW_SHIFT(ch)) &
>                                 SYS_REG_DBW_MASK;
>                         bg = (dbw == 2) ? 2 : 1;
> @@ -176,9 +177,11 @@ size_t rockchip_sdram_size(phys_addr_t reg)
>          *   2. update board_get_usable_ram_top() and dram_init_banksize()
>          *   to reserve memory for peripheral space after previous update.
>          */
> +
> +#ifndef __aarch64__

if (!IS_ENABLED(CONFIG_ARM64))

>         if (size_mb > (SDRAM_MAX_SIZE >> 20))
>                 size_mb = (SDRAM_MAX_SIZE >> 20);
> -
> +#endif
>         return (size_t)size_mb << 20;
>  }
>
> @@ -208,6 +211,10 @@ int dram_init(void)
>  ulong board_get_usable_ram_top(ulong total_size)
>  {
>         unsigned long top = CONFIG_SYS_SDRAM_BASE + SDRAM_MAX_SIZE;
> -
> -       return (gd->ram_top > top) ? top : gd->ram_top;
> +#ifdef SDRAM_UPPER_ADDR_MIN

eek, what is going on here? Can you use C instead of the preprocessor?


> +       if (gd->ram_top > SDRAM_UPPER_ADDR_MIN)
> +               return gd->ram_top;
> +       else
> +#endif
> +               return (gd->ram_top > top) ? top : gd->ram_top;
>  }
> diff --git a/common/board_f.c b/common/board_f.c
> index a68760092ac1..933ba7aedac0 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -344,7 +344,14 @@ static int setup_dest_addr(void)
>  #endif
>         gd->ram_top = gd->ram_base + get_effective_memsize();
>         gd->ram_top = board_get_usable_ram_top(gd->mon_len);
> +#ifdef SDRAM_LOWER_ADDR_MAX
> +       if (gd->ram_top > SDRAM_LOWER_ADDR_MAX)
> +               gd->relocaddr = SDRAM_LOWER_ADDR_MAX;
> +       else
> +               gd->relocaddr = gd->ram_top;
> +#else
>         gd->relocaddr = gd->ram_top;
> +#endif
>         debug("Ram top: %08lX\n", (ulong)gd->ram_top);
>  #if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
>         /*
> diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h
> index 25d7c5cc8fff..8dd1b033017b 100644
> --- a/include/configs/rk3568_common.h
> +++ b/include/configs/rk3568_common.h
> @@ -27,6 +27,11 @@
>  #define CONFIG_SYS_SDRAM_BASE          0
>  #define SDRAM_MAX_SIZE                 0xf0000000
>
> +#ifdef CONFIG_OF_SYSTEM_SETUP
> +#define SDRAM_LOWER_ADDR_MAX           0xf0000000
> +#define SDRAM_UPPER_ADDR_MIN           0x100000000
> +#endif

These config files are going away, so either use Kconfig or device tree.

> +
>  #ifndef CONFIG_SPL_BUILD
>  #define ENV_MEM_LAYOUT_SETTINGS                \
>         "scriptaddr=0x00c00000\0"       \
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Peter Geis]

On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Peter,
>
> On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > The reset handler for rk3568 is missing its private data. This leads to
> > an abort when a reset is triggered.
> >
> > Add the missing dev_set_priv to the rk3568 clk driver.
> >
> > Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >  drivers/clk/rockchip/clk_rk3568.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
> > index d5e45e7602c7..c83ae22dc302 100644
> > --- a/drivers/clk/rockchip/clk_rk3568.c
> > +++ b/drivers/clk/rockchip/clk_rk3568.c
> > @@ -14,6 +14,7 @@
> >  #include <asm/arch-rockchip/clock.h>
> >  #include <asm/arch-rockchip/hardware.h>
> >  #include <asm/io.h>
> > +#include <dm/device-internal.h>
> >  #include <dm/lists.h>
> >  #include <dt-bindings/clock/rk3568-cru.h>
> >
> > @@ -2934,6 +2935,7 @@ static int rk3568_clk_bind(struct udevice *dev)
> >                                                     glb_srst_fst);
> >                 priv->glb_srst_snd_value = offsetof(struct rk3568_cru,
> >                                                     glb_srsr_snd);
> > +               dev_set_priv(sys_child, priv);
> >         }
> >
> >  #if CONFIG_IS_ENABLED(RESET_ROCKCHIP)
> > --
> > 2.25.1
> >
>
> You must not access priv data in the bind() handler as it doesn't
> exist yet. The malloc() in that function is incorrect.

Strange, this makes this driver match literally every other rockchip
clock driver.
A few examples:
https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/rockchip/clk_rk3399.c#L1431
https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/rockchip/clk_rk3368.c#L625
https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/rockchip/clk_rk3328.c#L827

It's funny though, I thought this should reside completely within the
clock driver, because the sysreset handler is touching private cru
registers and I planned on moving it here.
Then I found this in the other drivers and went with this method for
consistency.

>
> You certainly must not set the priv data in there. See the lifecycle
> docs in driver model for how things are supposed to work.
>
> You can use plat data, so could move glb_srst_fst_value and
> glb_srst_snd_value to struct rk3568_clk_plat, although oddly that
> struct only exists if OF_PLATDATA is used.
>
> Quite confusing.
>
> Regards,
> Simon

Re: [PATCH v1 07/11] rockchip: rk3568: add boot device detection

[Peter Geis]

On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Peter,
>
> On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > Enable rk3568 spl to detect which device it was booted from.
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >  arch/arm/mach-rockchip/rk3568/rk3568.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> I think a better way would be to put an option in the U-Boot config node, like:
>
> boot-devices = <&sdhci_0 &spi_flash &sd>;

This would certainly solve the problem of needing to constantly update
the boot devices each time a dts sync occurs.
I'm still learning my way around u-boot's method of handling
device-trees vs linux's method.

>
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index 4e23feb9417f..5f239d89a7a9 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -7,6 +7,7 @@
> >  #include <dm.h>
> >  #include <asm/armv8/mmu.h>
> >  #include <asm/io.h>
> > +#include <asm/arch-rockchip/bootrom.h>
> >  #include <asm/arch-rockchip/grf_rk3568.h>
> >  #include <asm/arch-rockchip/hardware.h>
> >  #include <dt-bindings/clock/rk3568-cru.h>
> > @@ -23,6 +24,7 @@
> >  #define SGRF_SOC_CON4                  0x10
> >  #define EMMC_HPROT_SECURE_CTRL         0x03
> >  #define SDMMC0_HPROT_SECURE_CTRL       0x01
> > +
> >  /* PMU_GRF_GPIO0D_IOMUX_L */
> >  enum {
> >         GPIO0D1_SHIFT           = 4,
> > @@ -43,6 +45,12 @@ enum {
> >         UART2_IO_SEL_M0         = 0,
> >  };
> >
> > +const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> > +       [BROM_BOOTSOURCE_EMMC] = "/sdhci@fe310000",
> > +       [BROM_BOOTSOURCE_SPINOR] = "/spi@fe300000/flash@0",
> > +       [BROM_BOOTSOURCE_SD] = "/mmc@fe2b0000",
> > +};
> > +
> >  static struct mm_region rk3568_mem_map[] = {
> >         {
> >                 .virt = 0x0UL,
> > --
> > 2.25.1
> >
>
> Regards,
> Simon

Re: [PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Peter Geis]

On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Peter,
>
> On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > Fixup the bootrom recovery mode code to function in spl, so we can
> > handle recovery mode in case u-boot loading is broken.
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >  arch/arm/mach-rockchip/Makefile        |  6 +++---
> >  arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
> >  arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
> >  3 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> > index 00aef0ecee6a..53aff25ce8f6 100644
> > --- a/arch/arm/mach-rockchip/Makefile
> > +++ b/arch/arm/mach-rockchip/Makefile
> > @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
> >
> >  obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
> >
> > -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> > -
> >  # Always include boot_mode.o, as we bypass it (i.e. turn it off)
> >  # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
> >  # we can have the preprocessor correctly recognise both 0x0 and 0
> >  # meaning "turn it off".
> > -obj-y += boot_mode.o
> > +obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
> > +
> > +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> >  obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
> >  obj-$(CONFIG_MISC_INIT_R) += misc.o
> >  endif
> > diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
> > index 1a1a887fc2cd..43cb369465a2 100644
> > --- a/arch/arm/mach-rockchip/boot_mode.c
> > +++ b/arch/arm/mach-rockchip/boot_mode.c
> > @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
> >         ret = -ENODEV;
> >         uclass_foreach_dev(dev, uc) {
> >                 if (!strncmp(dev->name, "saradc", 6)) {
> > -                       ret = adc_channel_single_shot(dev->name, 1, &val);
> > +                       ret = adc_channel_single_shot(dev->name, 0, &val);
> >                         break;
> >                 }
> >         }
> > @@ -89,6 +89,7 @@ int setup_boot_mode(void)
> >         boot_mode = readl(reg);
> >         debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
> >
> > +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
>
> Can you use if() ?
>
> >         /* Clear boot mode */
> >         writel(BOOT_NORMAL, reg);
> >
> > @@ -102,6 +103,7 @@ int setup_boot_mode(void)
> >                 env_set("preboot", "setenv preboot; ums mmc 0");
> >                 break;
> >         }
> > +#endif
> >
> >         return 0;
> >  }
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index 22eeb77d41fa..4e23feb9417f 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -104,3 +104,26 @@ int arch_cpu_init(void)
> >  #endif
> >         return 0;
> >  }
> > +
> > +#ifdef CONFIG_SPL_BUILD
>
> Do you need that?

I intended this to fire only once during SPL, but I suppose it
wouldn't make much of a difference if it fired again.

>
> > +
> > +void __weak led_setup(void)
> > +{
> > +}
> > +
> > +void spl_board_init(void)
> > +{
> > +       led_setup();
> > +
> > +#if defined(SPL_DM_REGULATOR)
>
> You need a CONFIG_ prefix there.
>
> But better:
>
> if (CONFIG_IS_ENABLED(DM_REGULATOR))

Thanks!

>
> > +       /*
> > +        * Turning the eMMC and SPI back on (if disabled via the Qseven
> > +        * BIOS_ENABLE) signal is done through a always-on regulator).
> > +        */
> > +       if (regulators_enable_boot_on(false))
> > +               debug("%s: Cannot enable boot on regulator\n", __func__);
> > +#endif
> > +
> > +       setup_boot_mode();
> > +}
> > +#endif
> > --
> > 2.25.1
> >
>
> Regards,
> Simon

Re: [PATCH v1 10/11] rockchip: rk3568: add dwc3 otg support

[Peter Geis]

On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
>
> On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > Add the required platform data to the rk3568 chip config, in order to
> > support dwc3 otg on this chip.
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >  arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Seems that this info should be in the device tree.

I agree, but unless I'm missing something it seems the dwc3 otg code
can only take platform data right now.

Re: [PATCH v1 11/11] [RFC] rockchip: rk356x: attempt to fix ram detection

[Peter Geis]

On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Peter,
>
> On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > This patch attempts to fix ram detection on rk3568.
> > Prior to this, the rk3568 incorrectly detected 8gb ram as 2gb.
> > On top of this, the board panics when u-boot accesses ram above 4gb.
> >
> > Fix this by correcting ram detection in hopefully a backwards compatable
> > way, and extend board_f.c to enforce an upper limit on the ram u-boot
> > uses.
> > This allows us to limit the ram u-boot accesses, while passing the
> > correctly detected size to follow on software (eg linux).
> >
> > This has been tested on rk3566 2gb, 4gb, and 8gb configurations, as well
> > as rk3399 4gb configurations.
> >
> > I do not have other configurations available, and I do not have the
> > insights into rockchip ram handling to tell if this is the correct way
> > to go about this.
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >  arch/arm/mach-rockchip/Kconfig         |  1 +
> >  arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
> >  arch/arm/mach-rockchip/sdram.c         | 19 +++++++++++------
> >  common/board_f.c                       |  7 +++++++
> >  include/configs/rk3568_common.h        |  5 +++++
> >  5 files changed, 55 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> > index 92f35309e4a6..58393cc623f8 100644
> > --- a/arch/arm/mach-rockchip/Kconfig
> > +++ b/arch/arm/mach-rockchip/Kconfig
> > @@ -264,6 +264,7 @@ config ROCKCHIP_RK3568
> >         select SYSCON
> >         select BOARD_LATE_INIT
> >         imply ROCKCHIP_COMMON_BOARD
> > +       imply OF_SYSTEM_SETUP
> >         help
> >           The Rockchip RK3568 is a ARM-based SoC with quad-core Cortex-A55,
> >           including NEON and GPU, 512K L3 cache, Mali-G52 based graphics,
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index ef6bc67a88b0..8d2a59bc649d 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -5,6 +5,7 @@
> >
> >  #include <common.h>
> >  #include <dm.h>
> > +#include <fdt_support.h>
> >  #include <asm/armv8/mmu.h>
> >  #include <asm/io.h>
> >  #include <asm/arch-rockchip/bootrom.h>
> > @@ -187,3 +188,31 @@ int board_usb_init(int index, enum usb_init_type init)
> >  #endif /* CONFIG_USB_DWC3_GADGET */
> >
> >  #endif /* CONFIG_USB_GADGET */
> > +
> > +#ifdef CONFIG_OF_SYSTEM_SETUP
> > +int ft_system_setup(void *blob, struct bd_info *bd)
> > +{
> > +       int ret;
> > +       int areas = 1;
> > +       u64 start[2], size[2];
> > +
> > +       /* Reserve the io address space. */
> > +       if (gd->ram_top > SDRAM_UPPER_ADDR_MIN) {
>
> Can you put SDRAM_UPPER_ADDR_MIN and SDRAM_LOWER_ADDR_MAX into the
> device tree, perhaps?

This is the part of the patch series that gets really cludgy and I'm
not proud of it.
Essentially this is a cleaner implementation of what downstream does.
The issue is u-boot panics if it tries to access ram above 4G on this chip.
Combined with the MMIO address space below 4G, the available memory
map gets to be quite a mess.
I did try to keep it backwards compatible with the previous chips however.
The rk356x has 2G, 4G, and 8G boards in the wild.

I'll play a bit with a device tree method, to see if I can make it
make sense though.

>
>
> > +               start[0] = gd->bd->bi_dram[0].start;
> > +               size[0] = SDRAM_LOWER_ADDR_MAX - gd->bd->bi_dram[0].start;
> > +
> > +               /* Add the upper 4GB address space */
> > +               start[1] = SDRAM_UPPER_ADDR_MIN;
> > +               size[1] = gd->ram_top - SDRAM_UPPER_ADDR_MIN;
> > +               areas = 2;
> > +
> > +               ret = fdt_set_usable_memory(blob, start, size, areas);
> > +               if (ret) {
> > +                       printf("Cannot set usable memory\n");
> > +                       return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +};
> > +#endif
> > diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
> > index 705ec7ba6450..52974e6dc333 100644
> > --- a/arch/arm/mach-rockchip/sdram.c
> > +++ b/arch/arm/mach-rockchip/sdram.c
> > @@ -3,6 +3,8 @@
> >   * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> >   */
> >
> > +#define DEBUG
> > +
> >  #include <common.h>
> >  #include <dm.h>
> >  #include <init.h>
> > @@ -98,8 +100,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> >                           SYS_REG_COL_MASK);
> >                 cs1_col = cs0_col;
> >                 bk = 3 - ((sys_reg2 >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
> > -               if ((sys_reg3 >> SYS_REG_VERSION_SHIFT &
> > -                    SYS_REG_VERSION_MASK) == 0x2) {
> > +               if ((sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) >= 0x2) {
> >                         cs1_col = 9 + (sys_reg3 >> SYS_REG_CS1_COL_SHIFT(ch) &
> >                                   SYS_REG_CS1_COL_MASK);
> >                         if (((sys_reg3 >> SYS_REG_EXTEND_CS0_ROW_SHIFT(ch) &
> > @@ -136,7 +137,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> >                         SYS_REG_BW_MASK));
> >                 row_3_4 = sys_reg2 >> SYS_REG_ROW_3_4_SHIFT(ch) &
> >                         SYS_REG_ROW_3_4_MASK;
> > -               if (dram_type == DDR4) {
> > +               if ((dram_type == DDR4) && (sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) != 0x3){
>
> Space before {
>
> Also can you please add a comment as to what this is doing?

Thanks!
Indeed, I can. For reference here, it's retaining backwards
compatibility with older versions of the ram controller, but the new
ram controller ends up doubling the available ram if this is enabled.

>
> >                         dbw = (sys_reg2 >> SYS_REG_DBW_SHIFT(ch)) &
> >                                 SYS_REG_DBW_MASK;
> >                         bg = (dbw == 2) ? 2 : 1;
> > @@ -176,9 +177,11 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> >          *   2. update board_get_usable_ram_top() and dram_init_banksize()
> >          *   to reserve memory for peripheral space after previous update.
> >          */
> > +
> > +#ifndef __aarch64__
>
> if (!IS_ENABLED(CONFIG_ARM64))

Thanks!

>
> >         if (size_mb > (SDRAM_MAX_SIZE >> 20))
> >                 size_mb = (SDRAM_MAX_SIZE >> 20);
> > -
> > +#endif
> >         return (size_t)size_mb << 20;
> >  }
> >
> > @@ -208,6 +211,10 @@ int dram_init(void)
> >  ulong board_get_usable_ram_top(ulong total_size)
> >  {
> >         unsigned long top = CONFIG_SYS_SDRAM_BASE + SDRAM_MAX_SIZE;
> > -
> > -       return (gd->ram_top > top) ? top : gd->ram_top;
> > +#ifdef SDRAM_UPPER_ADDR_MIN
>
> eek, what is going on here? Can you use C instead of the preprocessor?

I was trying to keep to current convention in u-boot (which I admit is
uncomfortable coming from linux).
This is clamping the max available u-boot ram to SDRAM_UPPER_ADDR_MIN,
if it is set.
This is necessary because of the above panic issue, and is very much a hack.
Essentially I attempted to make this affect nothing if it isn't set,
thus if it isn't set boards that have 8GB will detect as 2GB.

The way rockchip currently clamps the ram to less than the MMIO space
feels really slimy too, but I didn't want to try and touch it too much
and risk breaking things.

>
>
> > +       if (gd->ram_top > SDRAM_UPPER_ADDR_MIN)
> > +               return gd->ram_top;
> > +       else
> > +#endif
> > +               return (gd->ram_top > top) ? top : gd->ram_top;
> >  }
> > diff --git a/common/board_f.c b/common/board_f.c
> > index a68760092ac1..933ba7aedac0 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -344,7 +344,14 @@ static int setup_dest_addr(void)
> >  #endif
> >         gd->ram_top = gd->ram_base + get_effective_memsize();
> >         gd->ram_top = board_get_usable_ram_top(gd->mon_len);
> > +#ifdef SDRAM_LOWER_ADDR_MAX
> > +       if (gd->ram_top > SDRAM_LOWER_ADDR_MAX)
> > +               gd->relocaddr = SDRAM_LOWER_ADDR_MAX;
> > +       else
> > +               gd->relocaddr = gd->ram_top;
> > +#else
> >         gd->relocaddr = gd->ram_top;
> > +#endif
> >         debug("Ram top: %08lX\n", (ulong)gd->ram_top);
> >  #if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
> >         /*
> > diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h
> > index 25d7c5cc8fff..8dd1b033017b 100644
> > --- a/include/configs/rk3568_common.h
> > +++ b/include/configs/rk3568_common.h
> > @@ -27,6 +27,11 @@
> >  #define CONFIG_SYS_SDRAM_BASE          0
> >  #define SDRAM_MAX_SIZE                 0xf0000000
> >
> > +#ifdef CONFIG_OF_SYSTEM_SETUP
> > +#define SDRAM_LOWER_ADDR_MAX           0xf0000000
> > +#define SDRAM_UPPER_ADDR_MIN           0x100000000
> > +#endif
>
> These config files are going away, so either use Kconfig or device tree.

Noted, I'm not sure how this could fit in a sane way to a device tree,
so Kconfig is probably the better option.

>
> > +
> >  #ifndef CONFIG_SPL_BUILD
> >  #define ENV_MEM_LAYOUT_SETTINGS                \
> >         "scriptaddr=0x00c00000\0"       \
> > --
> > 2.25.1
> >
>

This series was meant to expose the various flaws I'm working around
with the rk356x series right now.
This patch in particular was to expose the current issues with
rockchip ram detection.
It extended the already hacky method of handling this, mostly because
I don't have the internal knowledge of how Rockchip ram controllers
work.
On top of that I really don't like that u-boot can't touch above 4G
without a panic, but I definitely don't have enough knowledge of
u-boot's inner workings to fix it.

> Regards,
> Simon

Thanks for the review!
Peter

Re: [PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Simon Glass]

Hi Peter,

On Fri, 11 Mar 2022 at 20:32, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Peter,
> >
> > On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > The reset handler for rk3568 is missing its private data. This leads to
> > > an abort when a reset is triggered.
> > >
> > > Add the missing dev_set_priv to the rk3568 clk driver.
> > >
> > > Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
> > >
> > > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > > ---
> > >  drivers/clk/rockchip/clk_rk3568.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
> > > index d5e45e7602c7..c83ae22dc302 100644
> > > --- a/drivers/clk/rockchip/clk_rk3568.c
> > > +++ b/drivers/clk/rockchip/clk_rk3568.c
> > > @@ -14,6 +14,7 @@
> > >  #include <asm/arch-rockchip/clock.h>
> > >  #include <asm/arch-rockchip/hardware.h>
> > >  #include <asm/io.h>
> > > +#include <dm/device-internal.h>
> > >  #include <dm/lists.h>
> > >  #include <dt-bindings/clock/rk3568-cru.h>
> > >
> > > @@ -2934,6 +2935,7 @@ static int rk3568_clk_bind(struct udevice *dev)
> > >                                                     glb_srst_fst);
> > >                 priv->glb_srst_snd_value = offsetof(struct rk3568_cru,
> > >                                                     glb_srsr_snd);
> > > +               dev_set_priv(sys_child, priv);
> > >         }
> > >
> > >  #if CONFIG_IS_ENABLED(RESET_ROCKCHIP)
> > > --
> > > 2.25.1
> > >
> >
> > You must not access priv data in the bind() handler as it doesn't
> > exist yet. The malloc() in that function is incorrect.
>
> Strange, this makes this driver match literally every other rockchip
> clock driver.
> A few examples:
> https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/rockchip/clk_rk3399.c#L1431
> https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/rockchip/clk_rk3368.c#L625
> https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/rockchip/clk_rk3328.c#L827

Oops.

>
> It's funny though, I thought this should reside completely within the
> clock driver, because the sysreset handler is touching private cru
> registers and I planned on moving it here.
> Then I found this in the other drivers and went with this method for
> consistency.

I think it is OK for the sysreset driver to touch the cru, if it needs
to. We sometimes find we have multiple drivers for a single IP block,
particular the 'core' ones.


>
> >
> > You certainly must not set the priv data in there. See the lifecycle
> > docs in driver model for how things are supposed to work.
> >
> > You can use plat data, so could move glb_srst_fst_value and
> > glb_srst_snd_value to struct rk3568_clk_plat, although oddly that
> > struct only exists if OF_PLATDATA is used.
> >
> > Quite confusing.

Regards,
Simon

Re: [PATCH v1 11/11] [RFC] rockchip: rk356x: attempt to fix ram detection

[Simon Glass]

Hi Peter,

On Fri, 11 Mar 2022 at 20:54, Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Fri, Mar 11, 2022 at 9:25 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Peter,
> >
> > On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > This patch attempts to fix ram detection on rk3568.
> > > Prior to this, the rk3568 incorrectly detected 8gb ram as 2gb.
> > > On top of this, the board panics when u-boot accesses ram above 4gb.
> > >
> > > Fix this by correcting ram detection in hopefully a backwards compatable
> > > way, and extend board_f.c to enforce an upper limit on the ram u-boot
> > > uses.
> > > This allows us to limit the ram u-boot accesses, while passing the
> > > correctly detected size to follow on software (eg linux).
> > >
> > > This has been tested on rk3566 2gb, 4gb, and 8gb configurations, as well
> > > as rk3399 4gb configurations.
> > >
> > > I do not have other configurations available, and I do not have the
> > > insights into rockchip ram handling to tell if this is the correct way
> > > to go about this.
> > >
> > > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > > ---
> > >  arch/arm/mach-rockchip/Kconfig         |  1 +
> > >  arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++
> > >  arch/arm/mach-rockchip/sdram.c         | 19 +++++++++++------
> > >  common/board_f.c                       |  7 +++++++
> > >  include/configs/rk3568_common.h        |  5 +++++
> > >  5 files changed, 55 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> > > index 92f35309e4a6..58393cc623f8 100644
> > > --- a/arch/arm/mach-rockchip/Kconfig
> > > +++ b/arch/arm/mach-rockchip/Kconfig
> > > @@ -264,6 +264,7 @@ config ROCKCHIP_RK3568
> > >         select SYSCON
> > >         select BOARD_LATE_INIT
> > >         imply ROCKCHIP_COMMON_BOARD
> > > +       imply OF_SYSTEM_SETUP
> > >         help
> > >           The Rockchip RK3568 is a ARM-based SoC with quad-core Cortex-A55,
> > >           including NEON and GPU, 512K L3 cache, Mali-G52 based graphics,
> > > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > > index ef6bc67a88b0..8d2a59bc649d 100644
> > > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > > @@ -5,6 +5,7 @@
> > >
> > >  #include <common.h>
> > >  #include <dm.h>
> > > +#include <fdt_support.h>
> > >  #include <asm/armv8/mmu.h>
> > >  #include <asm/io.h>
> > >  #include <asm/arch-rockchip/bootrom.h>
> > > @@ -187,3 +188,31 @@ int board_usb_init(int index, enum usb_init_type init)
> > >  #endif /* CONFIG_USB_DWC3_GADGET */
> > >
> > >  #endif /* CONFIG_USB_GADGET */
> > > +
> > > +#ifdef CONFIG_OF_SYSTEM_SETUP
> > > +int ft_system_setup(void *blob, struct bd_info *bd)
> > > +{
> > > +       int ret;
> > > +       int areas = 1;
> > > +       u64 start[2], size[2];
> > > +
> > > +       /* Reserve the io address space. */
> > > +       if (gd->ram_top > SDRAM_UPPER_ADDR_MIN) {
> >
> > Can you put SDRAM_UPPER_ADDR_MIN and SDRAM_LOWER_ADDR_MAX into the
> > device tree, perhaps?
>
> This is the part of the patch series that gets really cludgy and I'm
> not proud of it.
> Essentially this is a cleaner implementation of what downstream does.
> The issue is u-boot panics if it tries to access ram above 4G on this chip.
> Combined with the MMIO address space below 4G, the available memory
> map gets to be quite a mess.
> I did try to keep it backwards compatible with the previous chips however.
> The rk356x has 2G, 4G, and 8G boards in the wild.
>
> I'll play a bit with a device tree method, to see if I can make it
> make sense though.
>
> >
> >
> > > +               start[0] = gd->bd->bi_dram[0].start;
> > > +               size[0] = SDRAM_LOWER_ADDR_MAX - gd->bd->bi_dram[0].start;
> > > +
> > > +               /* Add the upper 4GB address space */
> > > +               start[1] = SDRAM_UPPER_ADDR_MIN;
> > > +               size[1] = gd->ram_top - SDRAM_UPPER_ADDR_MIN;
> > > +               areas = 2;
> > > +
> > > +               ret = fdt_set_usable_memory(blob, start, size, areas);
> > > +               if (ret) {
> > > +                       printf("Cannot set usable memory\n");
> > > +                       return ret;
> > > +               }
> > > +       }
> > > +
> > > +       return 0;
> > > +};
> > > +#endif
> > > diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
> > > index 705ec7ba6450..52974e6dc333 100644
> > > --- a/arch/arm/mach-rockchip/sdram.c
> > > +++ b/arch/arm/mach-rockchip/sdram.c
> > > @@ -3,6 +3,8 @@
> > >   * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> > >   */
> > >
> > > +#define DEBUG
> > > +
> > >  #include <common.h>
> > >  #include <dm.h>
> > >  #include <init.h>
> > > @@ -98,8 +100,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> > >                           SYS_REG_COL_MASK);
> > >                 cs1_col = cs0_col;
> > >                 bk = 3 - ((sys_reg2 >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
> > > -               if ((sys_reg3 >> SYS_REG_VERSION_SHIFT &
> > > -                    SYS_REG_VERSION_MASK) == 0x2) {
> > > +               if ((sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) >= 0x2) {
> > >                         cs1_col = 9 + (sys_reg3 >> SYS_REG_CS1_COL_SHIFT(ch) &
> > >                                   SYS_REG_CS1_COL_MASK);
> > >                         if (((sys_reg3 >> SYS_REG_EXTEND_CS0_ROW_SHIFT(ch) &
> > > @@ -136,7 +137,7 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> > >                         SYS_REG_BW_MASK));
> > >                 row_3_4 = sys_reg2 >> SYS_REG_ROW_3_4_SHIFT(ch) &
> > >                         SYS_REG_ROW_3_4_MASK;
> > > -               if (dram_type == DDR4) {
> > > +               if ((dram_type == DDR4) && (sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) != 0x3){
> >
> > Space before {
> >
> > Also can you please add a comment as to what this is doing?
>
> Thanks!
> Indeed, I can. For reference here, it's retaining backwards
> compatibility with older versions of the ram controller, but the new
> ram controller ends up doubling the available ram if this is enabled.
>
> >
> > >                         dbw = (sys_reg2 >> SYS_REG_DBW_SHIFT(ch)) &
> > >                                 SYS_REG_DBW_MASK;
> > >                         bg = (dbw == 2) ? 2 : 1;
> > > @@ -176,9 +177,11 @@ size_t rockchip_sdram_size(phys_addr_t reg)
> > >          *   2. update board_get_usable_ram_top() and dram_init_banksize()
> > >          *   to reserve memory for peripheral space after previous update.
> > >          */
> > > +
> > > +#ifndef __aarch64__
> >
> > if (!IS_ENABLED(CONFIG_ARM64))
>
> Thanks!
>
> >
> > >         if (size_mb > (SDRAM_MAX_SIZE >> 20))
> > >                 size_mb = (SDRAM_MAX_SIZE >> 20);
> > > -
> > > +#endif
> > >         return (size_t)size_mb << 20;
> > >  }
> > >
> > > @@ -208,6 +211,10 @@ int dram_init(void)
> > >  ulong board_get_usable_ram_top(ulong total_size)
> > >  {
> > >         unsigned long top = CONFIG_SYS_SDRAM_BASE + SDRAM_MAX_SIZE;
> > > -
> > > -       return (gd->ram_top > top) ? top : gd->ram_top;
> > > +#ifdef SDRAM_UPPER_ADDR_MIN
> >
> > eek, what is going on here? Can you use C instead of the preprocessor?
>
> I was trying to keep to current convention in u-boot (which I admit is
> uncomfortable coming from linux).
> This is clamping the max available u-boot ram to SDRAM_UPPER_ADDR_MIN,
> if it is set.
> This is necessary because of the above panic issue, and is very much a hack.
> Essentially I attempted to make this affect nothing if it isn't set,
> thus if it isn't set boards that have 8GB will detect as 2GB.
>
> The way rockchip currently clamps the ram to less than the MMIO space
> feels really slimy too, but I didn't want to try and touch it too much
> and risk breaking things.

+Tom Rini

Well it is (I believe) OK to have #defines in files like asm/system.h
that are not CONFIG options. So you could move them there is there is
a separate system.h for each SoC. But if not, then Kconfig is better.

>
> >
> >
> > > +       if (gd->ram_top > SDRAM_UPPER_ADDR_MIN)
> > > +               return gd->ram_top;
> > > +       else
> > > +#endif
> > > +               return (gd->ram_top > top) ? top : gd->ram_top;
> > >  }
> > > diff --git a/common/board_f.c b/common/board_f.c
> > > index a68760092ac1..933ba7aedac0 100644
> > > --- a/common/board_f.c
> > > +++ b/common/board_f.c
> > > @@ -344,7 +344,14 @@ static int setup_dest_addr(void)
> > >  #endif
> > >         gd->ram_top = gd->ram_base + get_effective_memsize();
> > >         gd->ram_top = board_get_usable_ram_top(gd->mon_len);
> > > +#ifdef SDRAM_LOWER_ADDR_MAX
> > > +       if (gd->ram_top > SDRAM_LOWER_ADDR_MAX)
> > > +               gd->relocaddr = SDRAM_LOWER_ADDR_MAX;
> > > +       else
> > > +               gd->relocaddr = gd->ram_top;
> > > +#else
> > >         gd->relocaddr = gd->ram_top;
> > > +#endif
> > >         debug("Ram top: %08lX\n", (ulong)gd->ram_top);
> > >  #if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
> > >         /*
> > > diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h
> > > index 25d7c5cc8fff..8dd1b033017b 100644
> > > --- a/include/configs/rk3568_common.h
> > > +++ b/include/configs/rk3568_common.h
> > > @@ -27,6 +27,11 @@
> > >  #define CONFIG_SYS_SDRAM_BASE          0
> > >  #define SDRAM_MAX_SIZE                 0xf0000000
> > >
> > > +#ifdef CONFIG_OF_SYSTEM_SETUP
> > > +#define SDRAM_LOWER_ADDR_MAX           0xf0000000
> > > +#define SDRAM_UPPER_ADDR_MIN           0x100000000
> > > +#endif
> >
> > These config files are going away, so either use Kconfig or device tree.
>
> Noted, I'm not sure how this could fit in a sane way to a device tree,
> so Kconfig is probably the better option.

It could perhaps be a property in the dram node, like
u-boot,sdram-range = /bits 64/ <0xf0000000 0x100000000>

>
> >
> > > +
> > >  #ifndef CONFIG_SPL_BUILD
> > >  #define ENV_MEM_LAYOUT_SETTINGS                \
> > >         "scriptaddr=0x00c00000\0"       \
> > > --
> > > 2.25.1
> > >
> >
>
> This series was meant to expose the various flaws I'm working around
> with the rk356x series right now.
> This patch in particular was to expose the current issues with
> rockchip ram detection.
> It extended the already hacky method of handling this, mostly because
> I don't have the internal knowledge of how Rockchip ram controllers
> work.
> On top of that I really don't like that u-boot can't touch above 4G
> without a panic, but I definitely don't have enough knowledge of
> u-boot's inner workings to fix it.

I'm not sure about that one.

[..]

Regards,
Simon

Re: [PATCH v1 00/11] rockchip fixes and extend rk3568 support

[Kever Yang]

+ Joseph Chen

On 2022/2/22 09:31, Peter Geis wrote:
> to: Simon Glass <sjg@chromium.org>
> to: Philipp Tomsich <philipp.tomsich@vrull.eu>
> to: Kever Yang <kever.yang@rock-chips.com>
> to: Lukasz Majewski <lukma@denx.de>
> to: Sean Anderson <seanga2@gmail.com>
> to: Peng Fan <peng.fan@nxp.com>
> to: Jaehoon Chung <jh80.chung@samsung.com>
> to: Heiko Stübner <heiko@sntech.de>
> cc: u-boot@lists.denx.de
>
> Good Evening,
>
> The following is a few patches for rockchip mainline u-boot support.
> Patches 1-3 are fixes for the rk3568 reset handler, rockchip emmc dma to
> sram, and building the rockchip-sfc driver.
> Patch 4 adds a sanity check for the minimum sfc frequency.
> Patch 5 and 6 add adc support to spl and enable the rockchip recovery
> handler in spl, before attempting to load u-boot.
> Patch 7 enables rk3568 spl bootrom device detection.
> Patch 8 enables automatic clock gating and other power saving features
> on rk3568, which solves the chip running hotter compared to downstream.
> Patch 9 and 10 move the dwc3 platform data to the chip specific code and
> enable dwc3 otg support on rk3568.
> Patch 11 is an RFC patch for fixing ram detection on rk3568. Downstream
> goes about this a different way, where they implemented a special
> library to handle this.
>
> Please review and *especially test* patch 11.
>
> Very Respectfully,
> Peter Geis
>
> Peter Geis (11):
>    clk: rockchip: rk3568: fix reset handler
>    mmc: sdhci: allow disabling sdma in spl
>    spi: rockchip-sfc: fix building rockchip-sfc
>    spi: rockchip-sfc: sanity check minimum freq
>    spl: support adc drivers in spl
>    rockchip: handle bootrom recovery mode in spl
>    rockchip: rk3568: add boot device detection
>    rockchip: rk3568: enable automatic clock gating
>    rockchip: move dwc3 config to chip specific handler
>    rockchip: rk3568: add dwc3 otg support
>    [RFC] rockchip: rk356x: attempt to fix ram detection
>
>   arch/arm/mach-rockchip/Kconfig         |   1 +
>   arch/arm/mach-rockchip/Makefile        |   6 +-
>   arch/arm/mach-rockchip/board.c         |  24 ------
>   arch/arm/mach-rockchip/boot_mode.c     |   4 +-
>   arch/arm/mach-rockchip/rk3399/rk3399.c |  29 +++++++
>   arch/arm/mach-rockchip/rk3568/rk3568.c | 112 +++++++++++++++++++++++++
>   arch/arm/mach-rockchip/sdram.c         |  19 +++--
>   common/board_f.c                       |   7 ++
>   common/spl/Kconfig                     |   5 ++
>   drivers/Makefile                       |   1 +
>   drivers/clk/rockchip/clk_rk3568.c      |   2 +
>   drivers/mmc/Kconfig                    |   7 ++
>   drivers/mmc/sdhci.c                    |   6 +-
>   drivers/spi/rockchip_sfc.c             |   6 ++
>   include/configs/rk3568_common.h        |   5 ++
>   15 files changed, 197 insertions(+), 37 deletions(-)
>

Re: [PATCH v1 02/11] mmc: sdhci: allow disabling sdma in spl

[Kever Yang]

Hi Peter,


On 2022/2/22 09:31, Peter Geis wrote:
> Rockchip emmc devices have a similar issue to Rockchip dwmmc devices,
> where performing dma to sram causes errors with suspend/resume.
> Allow us to toggle sdma in spl for sdhci similar to adma support, so we
> can ensure dma is not used when loading the sram code.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever

> ---
>   drivers/mmc/Kconfig | 7 +++++++
>   drivers/mmc/sdhci.c | 6 +++---
>   2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index f04cc44e1973..1e4342285ce7 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -468,6 +468,13 @@ config MMC_SDHCI_SDMA
>   	  This enables support for the SDMA (Single Operation DMA) defined
>   	  in the SD Host Controller Standard Specification Version 1.00 .
>   
> +config SPL_MMC_SDHCI_SDMA
> +	bool "Support SDHCI SDMA in SPL"
> +	depends on MMC_SDHCI
> +	help
> +	  This enables support for the SDMA (Single Operation DMA) defined
> +	  in the SD Host Controller Standard Specification Version 1.00 in SPL.
> +
>   config MMC_SDHCI_ADMA
>   	bool "Support SDHCI ADMA2"
>   	depends on MMC_SDHCI
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 766e4a6b0c5e..6285e53d12a2 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -70,7 +70,7 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
>   	}
>   }
>   
> -#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
> +#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>   static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
>   			      int *is_aligned, int trans_bytes)
>   {
> @@ -177,7 +177,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
>   		}
>   	} while (!(stat & SDHCI_INT_DATA_END));
>   
> -#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
> +#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>   	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>   			 mmc_get_dma_dir(data));
>   #endif
> @@ -836,7 +836,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
>   #endif
>   	debug("%s, caps: 0x%x\n", __func__, caps);
>   
> -#ifdef CONFIG_MMC_SDHCI_SDMA
> +#if CONFIG_IS_ENABLED(MMC_SDHCI_SDMA)
>   	if ((caps & SDHCI_CAN_DO_SDMA)) {
>   		host->flags |= USE_SDMA;
>   	} else {

Re: [PATCH v1 03/11] spi: rockchip-sfc: fix building rockchip-sfc

[Kever Yang]

Hi Peter,

On 2022/2/22 09:31, Peter Geis wrote:
> The rockchip-sfc driver is missing an include to build correctly.


I think this driver builds OK and has been tested by other developer, 
could you share what kind of build environment do you using and what 
issue did you meet?


Thanks,

- Kever

>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>   drivers/spi/rockchip_sfc.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
> index e098addddcac..851a6482985b 100644
> --- a/drivers/spi/rockchip_sfc.c
> +++ b/drivers/spi/rockchip_sfc.c
> @@ -12,6 +12,7 @@
>   #include <bouncebuf.h>
>   #include <clk.h>
>   #include <dm.h>
> +#include <dm/device_compat.h>
>   #include <linux/bitops.h>
>   #include <linux/delay.h>
>   #include <linux/iopoll.h>

Re: [PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Kever Yang]

Hi Peter,


On 2022/2/22 09:31, Peter Geis wrote:
> The reset handler for rk3568 is missing its private data. This leads to
> an abort when a reset is triggered.
>
> Add the missing dev_set_priv to the rk3568 clk driver.
>
> Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever

> ---
>   drivers/clk/rockchip/clk_rk3568.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
> index d5e45e7602c7..c83ae22dc302 100644
> --- a/drivers/clk/rockchip/clk_rk3568.c
> +++ b/drivers/clk/rockchip/clk_rk3568.c
> @@ -14,6 +14,7 @@
>   #include <asm/arch-rockchip/clock.h>
>   #include <asm/arch-rockchip/hardware.h>
>   #include <asm/io.h>
> +#include <dm/device-internal.h>
>   #include <dm/lists.h>
>   #include <dt-bindings/clock/rk3568-cru.h>
>   
> @@ -2934,6 +2935,7 @@ static int rk3568_clk_bind(struct udevice *dev)
>   						    glb_srst_fst);
>   		priv->glb_srst_snd_value = offsetof(struct rk3568_cru,
>   						    glb_srsr_snd);
> +		dev_set_priv(sys_child, priv);
>   	}
>   
>   #if CONFIG_IS_ENABLED(RESET_ROCKCHIP)

Re: [PATCH v1 08/11] rockchip: rk3568: enable automatic clock gating

[Kever Yang]

Hi Peter,

On 2022/2/22 09:31, Peter Geis wrote:
> Enable automatic clock gating on rk3568, which solves a 7c temperature
> difference on SoQuartz compared to downstream.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>


Thanks,
- Kever
> ---
>   arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 5f239d89a7a9..0e0a7f5b54f2 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -25,6 +25,15 @@
>   #define EMMC_HPROT_SECURE_CTRL		0x03
>   #define SDMMC0_HPROT_SECURE_CTRL	0x01
>   
> +#define PMU_BASE_ADDR		0xfdd90000
> +#define PMU_NOC_AUTO_CON0	(0x70)
> +#define PMU_NOC_AUTO_CON1	(0x74)
> +#define EDP_PHY_GRF_BASE	0xfdcb0000
> +#define EDP_PHY_GRF_CON0	(EDP_PHY_GRF_BASE + 0x00)
> +#define EDP_PHY_GRF_CON10	(EDP_PHY_GRF_BASE + 0x28)
> +#define CPU_GRF_BASE		0xfdc30000
> +#define GRF_CORE_PVTPLL_CON0	(0x10)
> +
>   /* PMU_GRF_GPIO0D_IOMUX_L */
>   enum {
>   	GPIO0D1_SHIFT		= 4,
> @@ -99,6 +108,20 @@ void board_debug_uart_init(void)
>   int arch_cpu_init(void)
>   {
>   #ifdef CONFIG_SPL_BUILD
> +	/*
> +	 * When perform idle operation, corresponding clock can
> +	 * be opened or gated automatically.
> +	 */
> +	writel(0xffffffff, PMU_BASE_ADDR + PMU_NOC_AUTO_CON0);
> +	writel(0x000f000f, PMU_BASE_ADDR + PMU_NOC_AUTO_CON1);
> +
> +	/* Disable eDP phy by default */
> +	writel(0x00070007, EDP_PHY_GRF_CON10);
> +	writel(0x0ff10ff1, EDP_PHY_GRF_CON0);
> +
> +	/* Set core pvtpll ring length */
> +	writel(0x00ff002b, CPU_GRF_BASE + GRF_CORE_PVTPLL_CON0);
> +
>   	/* Set the emmc sdmmc0 to secure */
>   	rk_clrreg(SGRF_BASE + SGRF_SOC_CON4, (EMMC_HPROT_SECURE_CTRL << 11
>   		| SDMMC0_HPROT_SECURE_CTRL << 4));

Re: [PATCH v1 04/11] spi: rockchip-sfc: sanity check minimum freq

[Kever Yang]

+ Jon Lin,

Hi Jon,

     Please help to review this patch.


Thanks,
- Kever
On 2022/2/22 09:31, Peter Geis wrote:
> The rockchip-sfc driver sanity checks the maximum frequency, but not the
> minimum frequency.
> This causes the probe to fail when a frequency isn't defined, such as
> with `sf probe 0`.
> Clamp the minimum frequency to the rockchip default clock rate.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>   drivers/spi/rockchip_sfc.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
> index 851a6482985b..d0d2dc70a417 100644
> --- a/drivers/spi/rockchip_sfc.c
> +++ b/drivers/spi/rockchip_sfc.c
> @@ -164,6 +164,8 @@
>   /* DMA is only enabled for large data transmission */
>   #define SFC_DMA_TRANS_THRETHOLD		(0x40)
>   
> +#define SFC_MIN_SPEED		(24 * 1000 * 1000)
> +
>   /* Maximum clock values from datasheet suggest keeping clock value under
>    * 150MHz. No minimum or average value is suggested.
>    */
> @@ -596,6 +598,9 @@ static int rockchip_sfc_set_speed(struct udevice *bus, uint speed)
>   	if (speed > sfc->max_freq)
>   		speed = sfc->max_freq;
>   
> +	if (speed < SFC_MIN_SPEED)
> +		speed = SFC_MIN_SPEED;
> +
>   	if (speed == sfc->speed)
>   		return 0;
>   

Re: [PATCH v1 00/11] rockchip fixes and extend rk3568 support

[Kever Yang]

Hi Peter,

     Thanks for your patchset.

     Seems like some of the driver module author does not include in the 
patch cc list, I would suggest you to use scripts/get_maintainer.pl 
instead of add cc manually

tools/patman/patman is a very wonderful tool, you can have a try :)


Thanks,

- Kever

On 2022/2/22 09:31, Peter Geis wrote:
> to: Simon Glass <sjg@chromium.org>
> to: Philipp Tomsich <philipp.tomsich@vrull.eu>
> to: Kever Yang <kever.yang@rock-chips.com>
> to: Lukasz Majewski <lukma@denx.de>
> to: Sean Anderson <seanga2@gmail.com>
> to: Peng Fan <peng.fan@nxp.com>
> to: Jaehoon Chung <jh80.chung@samsung.com>
> to: Heiko Stübner <heiko@sntech.de>
> cc: u-boot@lists.denx.de
>
> Good Evening,
>
> The following is a few patches for rockchip mainline u-boot support.
> Patches 1-3 are fixes for the rk3568 reset handler, rockchip emmc dma to
> sram, and building the rockchip-sfc driver.
> Patch 4 adds a sanity check for the minimum sfc frequency.
> Patch 5 and 6 add adc support to spl and enable the rockchip recovery
> handler in spl, before attempting to load u-boot.
> Patch 7 enables rk3568 spl bootrom device detection.
> Patch 8 enables automatic clock gating and other power saving features
> on rk3568, which solves the chip running hotter compared to downstream.
> Patch 9 and 10 move the dwc3 platform data to the chip specific code and
> enable dwc3 otg support on rk3568.
> Patch 11 is an RFC patch for fixing ram detection on rk3568. Downstream
> goes about this a different way, where they implemented a special
> library to handle this.
>
> Please review and *especially test* patch 11.
>
> Very Respectfully,
> Peter Geis
>
> Peter Geis (11):
>    clk: rockchip: rk3568: fix reset handler
>    mmc: sdhci: allow disabling sdma in spl
>    spi: rockchip-sfc: fix building rockchip-sfc
>    spi: rockchip-sfc: sanity check minimum freq
>    spl: support adc drivers in spl
>    rockchip: handle bootrom recovery mode in spl
>    rockchip: rk3568: add boot device detection
>    rockchip: rk3568: enable automatic clock gating
>    rockchip: move dwc3 config to chip specific handler
>    rockchip: rk3568: add dwc3 otg support
>    [RFC] rockchip: rk356x: attempt to fix ram detection
>
>   arch/arm/mach-rockchip/Kconfig         |   1 +
>   arch/arm/mach-rockchip/Makefile        |   6 +-
>   arch/arm/mach-rockchip/board.c         |  24 ------
>   arch/arm/mach-rockchip/boot_mode.c     |   4 +-
>   arch/arm/mach-rockchip/rk3399/rk3399.c |  29 +++++++
>   arch/arm/mach-rockchip/rk3568/rk3568.c | 112 +++++++++++++++++++++++++
>   arch/arm/mach-rockchip/sdram.c         |  19 +++--
>   common/board_f.c                       |   7 ++
>   common/spl/Kconfig                     |   5 ++
>   drivers/Makefile                       |   1 +
>   drivers/clk/rockchip/clk_rk3568.c      |   2 +
>   drivers/mmc/Kconfig                    |   7 ++
>   drivers/mmc/sdhci.c                    |   6 +-
>   drivers/spi/rockchip_sfc.c             |   6 ++
>   include/configs/rk3568_common.h        |   5 ++
>   15 files changed, 197 insertions(+), 37 deletions(-)
>

Re: [PATCH v1 00/11] rockchip fixes and extend rk3568 support

[Jagan Teki]

On Tue, Feb 22, 2022 at 7:02 AM Peter Geis <pgwipeout@gmail.com> wrote:
>
> to: Simon Glass <sjg@chromium.org>
> to: Philipp Tomsich <philipp.tomsich@vrull.eu>
> to: Kever Yang <kever.yang@rock-chips.com>
> to: Lukasz Majewski <lukma@denx.de>
> to: Sean Anderson <seanga2@gmail.com>
> to: Peng Fan <peng.fan@nxp.com>
> to: Jaehoon Chung <jh80.chung@samsung.com>
> to: Heiko Stübner <heiko@sntech.de>
> cc: u-boot@lists.denx.de
>
> Good Evening,
>
> The following is a few patches for rockchip mainline u-boot support.
> Patches 1-3 are fixes for the rk3568 reset handler, rockchip emmc dma to
> sram, and building the rockchip-sfc driver.
> Patch 4 adds a sanity check for the minimum sfc frequency.
> Patch 5 and 6 add adc support to spl and enable the rockchip recovery
> handler in spl, before attempting to load u-boot.
> Patch 7 enables rk3568 spl bootrom device detection.
> Patch 8 enables automatic clock gating and other power saving features
> on rk3568, which solves the chip running hotter compared to downstream.
> Patch 9 and 10 move the dwc3 platform data to the chip specific code and
> enable dwc3 otg support on rk3568.
> Patch 11 is an RFC patch for fixing ram detection on rk3568. Downstream
> goes about this a different way, where they implemented a special
> library to handle this.
>
> Please review and *especially test* patch 11.
>
> Very Respectfully,
> Peter Geis
>
> Peter Geis (11):
>   clk: rockchip: rk3568: fix reset handler
>   mmc: sdhci: allow disabling sdma in spl
>   spi: rockchip-sfc: fix building rockchip-sfc
>   spi: rockchip-sfc: sanity check minimum freq
>   spl: support adc drivers in spl
>   rockchip: handle bootrom recovery mode in spl
>   rockchip: rk3568: add boot device detection
>   rockchip: rk3568: enable automatic clock gating
>   rockchip: move dwc3 config to chip specific handler
>   rockchip: rk3568: add dwc3 otg support
>   [RFC] rockchip: rk356x: attempt to fix ram detection

Better keep sending
1. drivers on respective mintainers. - and approve them
2. add rk3568 support as separate series by syncing Linux dts(i) from
a specific tag.

This way it is easier for maintainers to review it.

Thanks,
Jagan.

Re: [PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Kever Yang]

Hi Peter,

On 2022/2/22 09:31, Peter Geis wrote:
> Fixup the bootrom recovery mode code to function in spl, so we can
> handle recovery mode in case u-boot loading is broken.
I don't understand what do you mean about "Fixup the bootrom recovery 
mode code"?
I would like to make SPL simple and small enough so that it can fit all 
kinds of SoCs.
I believe  there are other modules to coming after saradc if we enable 
boot_mode in SPL,
the SPL may become bloatware later.
So please remove the boot mode relate patches so that other patches need 
by rk3568 can be
merge first.

Thanks,
- Kever
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>   arch/arm/mach-rockchip/Makefile        |  6 +++---
>   arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
>   arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
>   3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index 00aef0ecee6a..53aff25ce8f6 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
>   
>   obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
>   
> -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> -
>   # Always include boot_mode.o, as we bypass it (i.e. turn it off)
>   # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
>   # we can have the preprocessor correctly recognise both 0x0 and 0
>   # meaning "turn it off".
> -obj-y += boot_mode.o
> +obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
> +
> +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
>   obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
>   obj-$(CONFIG_MISC_INIT_R) += misc.o
>   endif
> diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
> index 1a1a887fc2cd..43cb369465a2 100644
> --- a/arch/arm/mach-rockchip/boot_mode.c
> +++ b/arch/arm/mach-rockchip/boot_mode.c
> @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
>   	ret = -ENODEV;
>   	uclass_foreach_dev(dev, uc) {
>   		if (!strncmp(dev->name, "saradc", 6)) {
> -			ret = adc_channel_single_shot(dev->name, 1, &val);
> +			ret = adc_channel_single_shot(dev->name, 0, &val);
>   			break;
>   		}
>   	}
> @@ -89,6 +89,7 @@ int setup_boot_mode(void)
>   	boot_mode = readl(reg);
>   	debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
>   
> +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
>   	/* Clear boot mode */
>   	writel(BOOT_NORMAL, reg);
>   
> @@ -102,6 +103,7 @@ int setup_boot_mode(void)
>   		env_set("preboot", "setenv preboot; ums mmc 0");
>   		break;
>   	}
> +#endif
>   
>   	return 0;
>   }
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 22eeb77d41fa..4e23feb9417f 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -104,3 +104,26 @@ int arch_cpu_init(void)
>   #endif
>   	return 0;
>   }
> +
> +#ifdef CONFIG_SPL_BUILD
> +
> +void __weak led_setup(void)
> +{
> +}
> +
> +void spl_board_init(void)
> +{
> +	led_setup();
> +
> +#if defined(SPL_DM_REGULATOR)
> +	/*
> +	 * Turning the eMMC and SPI back on (if disabled via the Qseven
> +	 * BIOS_ENABLE) signal is done through a always-on regulator).
> +	 */
> +	if (regulators_enable_boot_on(false))
> +		debug("%s: Cannot enable boot on regulator\n", __func__);
> +#endif
> +
> +	setup_boot_mode();
> +}
> +#endif

Re: [PATCH v1 03/11] spi: rockchip-sfc: fix building rockchip-sfc

[Peter Geis]

On Mon, Mar 14, 2022 at 4:47 AM Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Peter,

Good Morning,

>
> On 2022/2/22 09:31, Peter Geis wrote:
> > The rockchip-sfc driver is missing an include to build correctly.
>
>
> I think this driver builds OK and has been tested by other developer,
> could you share what kind of build environment do you using and what
> issue did you meet?

If you enable debugging, the dev_dbg functions are not included due to
header cleanups.
dm/device_compat.h provides them.

>
>
> Thanks,
>
> - Kever

Always,
Peter

>
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >   drivers/spi/rockchip_sfc.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
> > index e098addddcac..851a6482985b 100644
> > --- a/drivers/spi/rockchip_sfc.c
> > +++ b/drivers/spi/rockchip_sfc.c
> > @@ -12,6 +12,7 @@
> >   #include <bouncebuf.h>
> >   #include <clk.h>
> >   #include <dm.h>
> > +#include <dm/device_compat.h>
> >   #include <linux/bitops.h>
> >   #include <linux/delay.h>
> >   #include <linux/iopoll.h>

Re: [PATCH v1 00/11] rockchip fixes and extend rk3568 support

[Peter Geis]

On Mon, Mar 14, 2022 at 4:56 AM Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Peter,
>
>      Thanks for your patchset.
>
>      Seems like some of the driver module author does not include in the
> patch cc list, I would suggest you to use scripts/get_maintainer.pl
> instead of add cc manually

I do use scripts/get_maintainer.pl automatically, which fails
hilariously at the cover letter.
I then bring anyone who's listed as a maintainer into the cover letter
as well as the mailing lists manually by running it on the entire
patch series.
If someone is missing, then the maintainers file isn't grabbing them.
I saw a patch recently to help with that.

>
> tools/patman/patman is a very wonderful tool, you can have a try :)

I'll look into this!

>
>
> Thanks,
>
> - Kever

Always,
Peter

>
> On 2022/2/22 09:31, Peter Geis wrote:
> > to: Simon Glass <sjg@chromium.org>
> > to: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > to: Kever Yang <kever.yang@rock-chips.com>
> > to: Lukasz Majewski <lukma@denx.de>
> > to: Sean Anderson <seanga2@gmail.com>
> > to: Peng Fan <peng.fan@nxp.com>
> > to: Jaehoon Chung <jh80.chung@samsung.com>
> > to: Heiko Stübner <heiko@sntech.de>
> > cc: u-boot@lists.denx.de
> >
> > Good Evening,
> >
> > The following is a few patches for rockchip mainline u-boot support.
> > Patches 1-3 are fixes for the rk3568 reset handler, rockchip emmc dma to
> > sram, and building the rockchip-sfc driver.
> > Patch 4 adds a sanity check for the minimum sfc frequency.
> > Patch 5 and 6 add adc support to spl and enable the rockchip recovery
> > handler in spl, before attempting to load u-boot.
> > Patch 7 enables rk3568 spl bootrom device detection.
> > Patch 8 enables automatic clock gating and other power saving features
> > on rk3568, which solves the chip running hotter compared to downstream.
> > Patch 9 and 10 move the dwc3 platform data to the chip specific code and
> > enable dwc3 otg support on rk3568.
> > Patch 11 is an RFC patch for fixing ram detection on rk3568. Downstream
> > goes about this a different way, where they implemented a special
> > library to handle this.
> >
> > Please review and *especially test* patch 11.
> >
> > Very Respectfully,
> > Peter Geis
> >
> > Peter Geis (11):
> >    clk: rockchip: rk3568: fix reset handler
> >    mmc: sdhci: allow disabling sdma in spl
> >    spi: rockchip-sfc: fix building rockchip-sfc
> >    spi: rockchip-sfc: sanity check minimum freq
> >    spl: support adc drivers in spl
> >    rockchip: handle bootrom recovery mode in spl
> >    rockchip: rk3568: add boot device detection
> >    rockchip: rk3568: enable automatic clock gating
> >    rockchip: move dwc3 config to chip specific handler
> >    rockchip: rk3568: add dwc3 otg support
> >    [RFC] rockchip: rk356x: attempt to fix ram detection
> >
> >   arch/arm/mach-rockchip/Kconfig         |   1 +
> >   arch/arm/mach-rockchip/Makefile        |   6 +-
> >   arch/arm/mach-rockchip/board.c         |  24 ------
> >   arch/arm/mach-rockchip/boot_mode.c     |   4 +-
> >   arch/arm/mach-rockchip/rk3399/rk3399.c |  29 +++++++
> >   arch/arm/mach-rockchip/rk3568/rk3568.c | 112 +++++++++++++++++++++++++
> >   arch/arm/mach-rockchip/sdram.c         |  19 +++--
> >   common/board_f.c                       |   7 ++
> >   common/spl/Kconfig                     |   5 ++
> >   drivers/Makefile                       |   1 +
> >   drivers/clk/rockchip/clk_rk3568.c      |   2 +
> >   drivers/mmc/Kconfig                    |   7 ++
> >   drivers/mmc/sdhci.c                    |   6 +-
> >   drivers/spi/rockchip_sfc.c             |   6 ++
> >   include/configs/rk3568_common.h        |   5 ++
> >   15 files changed, 197 insertions(+), 37 deletions(-)
> >

Re: [PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Peter Geis]

On Mon, Mar 14, 2022 at 5:10 AM Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Peter,

Good Morning,

>
> On 2022/2/22 09:31, Peter Geis wrote:
> > Fixup the bootrom recovery mode code to function in spl, so we can
> > handle recovery mode in case u-boot loading is broken.
> I don't understand what do you mean about "Fixup the bootrom recovery
> mode code"?
> I would like to make SPL simple and small enough so that it can fit all
> kinds of SoCs.

This actually brings mainline's behavior into line with Rockchip's u-boot's.
Having SPL handle this also means if a broken u-boot is flashed, we
can recover it on devices that don't have removable storage.
Also, channel 1 is incorrect per Rockchip's documentation about the
maskrom key, following their reference designs channel 0 is always the
maskrom channel.

> I believe  there are other modules to coming after saradc if we enable
> boot_mode in SPL,
> the SPL may become bloatware later.

I added a toggle that allows turning on or off SARADC in SPL.

> So please remove the boot mode relate patches so that other patches need
> by rk3568 can be
> merge first.
>
> Thanks,
> - Kever

Always,
Peter

> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >   arch/arm/mach-rockchip/Makefile        |  6 +++---
> >   arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
> >   arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
> >   3 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> > index 00aef0ecee6a..53aff25ce8f6 100644
> > --- a/arch/arm/mach-rockchip/Makefile
> > +++ b/arch/arm/mach-rockchip/Makefile
> > @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
> >
> >   obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
> >
> > -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> > -
> >   # Always include boot_mode.o, as we bypass it (i.e. turn it off)
> >   # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
> >   # we can have the preprocessor correctly recognise both 0x0 and 0
> >   # meaning "turn it off".
> > -obj-y += boot_mode.o
> > +obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
> > +
> > +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> >   obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
> >   obj-$(CONFIG_MISC_INIT_R) += misc.o
> >   endif
> > diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
> > index 1a1a887fc2cd..43cb369465a2 100644
> > --- a/arch/arm/mach-rockchip/boot_mode.c
> > +++ b/arch/arm/mach-rockchip/boot_mode.c
> > @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
> >       ret = -ENODEV;
> >       uclass_foreach_dev(dev, uc) {
> >               if (!strncmp(dev->name, "saradc", 6)) {
> > -                     ret = adc_channel_single_shot(dev->name, 1, &val);
> > +                     ret = adc_channel_single_shot(dev->name, 0, &val);
> >                       break;
> >               }
> >       }
> > @@ -89,6 +89,7 @@ int setup_boot_mode(void)
> >       boot_mode = readl(reg);
> >       debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
> >
> > +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> >       /* Clear boot mode */
> >       writel(BOOT_NORMAL, reg);
> >
> > @@ -102,6 +103,7 @@ int setup_boot_mode(void)
> >               env_set("preboot", "setenv preboot; ums mmc 0");
> >               break;
> >       }
> > +#endif
> >
> >       return 0;
> >   }
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index 22eeb77d41fa..4e23feb9417f 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -104,3 +104,26 @@ int arch_cpu_init(void)
> >   #endif
> >       return 0;
> >   }
> > +
> > +#ifdef CONFIG_SPL_BUILD
> > +
> > +void __weak led_setup(void)
> > +{
> > +}
> > +
> > +void spl_board_init(void)
> > +{
> > +     led_setup();
> > +
> > +#if defined(SPL_DM_REGULATOR)
> > +     /*
> > +      * Turning the eMMC and SPI back on (if disabled via the Qseven
> > +      * BIOS_ENABLE) signal is done through a always-on regulator).
> > +      */
> > +     if (regulators_enable_boot_on(false))
> > +             debug("%s: Cannot enable boot on regulator\n", __func__);
> > +#endif
> > +
> > +     setup_boot_mode();
> > +}
> > +#endif

Re: [PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Philipp Tomsich]

On Tue, 22 Feb 2022 at 02:31, Peter Geis <pgwipeout@gmail.com> wrote:
>
> Fixup the bootrom recovery mode code to function in spl, so we can
> handle recovery mode in case u-boot loading is broken.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
>  arch/arm/mach-rockchip/Makefile        |  6 +++---
>  arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
>  arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index 00aef0ecee6a..53aff25ce8f6 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
>
>  obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
>
> -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> -
>  # Always include boot_mode.o, as we bypass it (i.e. turn it off)
>  # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
>  # we can have the preprocessor correctly recognise both 0x0 and 0
>  # meaning "turn it off".
> -obj-y += boot_mode.o
> +obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
> +
> +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
>  obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
>  obj-$(CONFIG_MISC_INIT_R) += misc.o
>  endif
> diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
> index 1a1a887fc2cd..43cb369465a2 100644
> --- a/arch/arm/mach-rockchip/boot_mode.c
> +++ b/arch/arm/mach-rockchip/boot_mode.c
> @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
>         ret = -ENODEV;
>         uclass_foreach_dev(dev, uc) {
>                 if (!strncmp(dev->name, "saradc", 6)) {
> -                       ret = adc_channel_single_shot(dev->name, 1, &val);
> +                       ret = adc_channel_single_shot(dev->name, 0, &val);

This looks like an unrelated change (i.e., doesn't correlate with the
commit message).  Should this go into a separate patch in the same
series?

>                         break;
>                 }
>         }
> @@ -89,6 +89,7 @@ int setup_boot_mode(void)
>         boot_mode = readl(reg);
>         debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
>
> +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
>         /* Clear boot mode */
>         writel(BOOT_NORMAL, reg);
>
> @@ -102,6 +103,7 @@ int setup_boot_mode(void)
>                 env_set("preboot", "setenv preboot; ums mmc 0");
>                 break;
>         }
> +#endif
>
>         return 0;
>  }
> diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> index 22eeb77d41fa..4e23feb9417f 100644
> --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> @@ -104,3 +104,26 @@ int arch_cpu_init(void)
>  #endif
>         return 0;
>  }
> +
> +#ifdef CONFIG_SPL_BUILD
> +
> +void __weak led_setup(void)
> +{
> +}
> +
> +void spl_board_init(void)
> +{
> +       led_setup();
> +
> +#if defined(SPL_DM_REGULATOR)
> +       /*
> +        * Turning the eMMC and SPI back on (if disabled via the Qseven
> +        * BIOS_ENABLE) signal is done through a always-on regulator).
> +        */
> +       if (regulators_enable_boot_on(false))
> +               debug("%s: Cannot enable boot on regulator\n", __func__);
> +#endif
> +
> +       setup_boot_mode();
> +}
> +#endif
> --
> 2.25.1
>

Re: [PATCH v1 06/11] rockchip: handle bootrom recovery mode in spl

[Peter Geis]

On Mon, Mar 14, 2022 at 7:59 AM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> On Tue, 22 Feb 2022 at 02:31, Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > Fixup the bootrom recovery mode code to function in spl, so we can
> > handle recovery mode in case u-boot loading is broken.
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >  arch/arm/mach-rockchip/Makefile        |  6 +++---
> >  arch/arm/mach-rockchip/boot_mode.c     |  4 +++-
> >  arch/arm/mach-rockchip/rk3568/rk3568.c | 23 +++++++++++++++++++++++
> >  3 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> > index 00aef0ecee6a..53aff25ce8f6 100644
> > --- a/arch/arm/mach-rockchip/Makefile
> > +++ b/arch/arm/mach-rockchip/Makefile
> > @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
> >
> >  obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
> >
> > -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> > -
> >  # Always include boot_mode.o, as we bypass it (i.e. turn it off)
> >  # inside of boot_mode.c when CONFIG_BOOT_MODE_REG is 0.  This way,
> >  # we can have the preprocessor correctly recognise both 0x0 and 0
> >  # meaning "turn it off".
> > -obj-y += boot_mode.o
> > +obj-$(CONFIG_ARCH_ROCKCHIP) += boot_mode.o
> > +
> > +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> >  obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
> >  obj-$(CONFIG_MISC_INIT_R) += misc.o
> >  endif
> > diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
> > index 1a1a887fc2cd..43cb369465a2 100644
> > --- a/arch/arm/mach-rockchip/boot_mode.c
> > +++ b/arch/arm/mach-rockchip/boot_mode.c
> > @@ -51,7 +51,7 @@ __weak int rockchip_dnl_key_pressed(void)
> >         ret = -ENODEV;
> >         uclass_foreach_dev(dev, uc) {
> >                 if (!strncmp(dev->name, "saradc", 6)) {
> > -                       ret = adc_channel_single_shot(dev->name, 1, &val);
> > +                       ret = adc_channel_single_shot(dev->name, 0, &val);
>
> This looks like an unrelated change (i.e., doesn't correlate with the
> commit message).  Should this go into a separate patch in the same
> series?

Makes sense, I can separate out this change.

>
> >                         break;
> >                 }
> >         }
> > @@ -89,6 +89,7 @@ int setup_boot_mode(void)
> >         boot_mode = readl(reg);
> >         debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
> >
> > +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> >         /* Clear boot mode */
> >         writel(BOOT_NORMAL, reg);
> >
> > @@ -102,6 +103,7 @@ int setup_boot_mode(void)
> >                 env_set("preboot", "setenv preboot; ums mmc 0");
> >                 break;
> >         }
> > +#endif
> >
> >         return 0;
> >  }
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index 22eeb77d41fa..4e23feb9417f 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -104,3 +104,26 @@ int arch_cpu_init(void)
> >  #endif
> >         return 0;
> >  }
> > +
> > +#ifdef CONFIG_SPL_BUILD
> > +
> > +void __weak led_setup(void)
> > +{
> > +}
> > +
> > +void spl_board_init(void)
> > +{
> > +       led_setup();
> > +
> > +#if defined(SPL_DM_REGULATOR)
> > +       /*
> > +        * Turning the eMMC and SPI back on (if disabled via the Qseven
> > +        * BIOS_ENABLE) signal is done through a always-on regulator).
> > +        */
> > +       if (regulators_enable_boot_on(false))
> > +               debug("%s: Cannot enable boot on regulator\n", __func__);
> > +#endif
> > +
> > +       setup_boot_mode();
> > +}
> > +#endif
> > --
> > 2.25.1
> >

Re: [PATCH v1 04/11] spi: rockchip-sfc: sanity check minimum freq

[Jon Lin]

在 2022/3/14 16:53, Kever Yang 写道:
> + Jon Lin,
> 
> Hi Jon,
> 
>      Please help to review this patch.
> 
> 
> Thanks,
> - Kever
> On 2022/2/22 09:31, Peter Geis wrote:
>> The rockchip-sfc driver sanity checks the maximum frequency, but not the
>> minimum frequency.
>> This causes the probe to fail when a frequency isn't defined, such as
>> with `sf probe 0`.
>> Clamp the minimum frequency to the rockchip default clock rate.
>>
>> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
>> ---
>>   drivers/spi/rockchip_sfc.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/spi/rockchip_sfc.c b/drivers/spi/rockchip_sfc.c
>> index 851a6482985b..d0d2dc70a417 100644
>> --- a/drivers/spi/rockchip_sfc.c
>> +++ b/drivers/spi/rockchip_sfc.c
>> @@ -164,6 +164,8 @@
>>   /* DMA is only enabled for large data transmission */
>>   #define SFC_DMA_TRANS_THRETHOLD        (0x40)
>> +#define SFC_MIN_SPEED        (24 * 1000 * 1000)
>> +
>>   /* Maximum clock values from datasheet suggest keeping clock value 
>> under
>>    * 150MHz. No minimum or average value is suggested.
>>    */
>> @@ -596,6 +598,9 @@ static int rockchip_sfc_set_speed(struct udevice 
>> *bus, uint speed)
>>       if (speed > sfc->max_freq)
>>           speed = sfc->max_freq;
>> +    if (speed < SFC_MIN_SPEED)
>> +        speed = SFC_MIN_SPEED;
>> +
>>       if (speed == sfc->speed)
>>           return 0;

As I know, spi-uclass.c set SPI_DEFAULT_SPEED_HZ as default value when 
detect the param speed is 0.

And I think, For SF probe, error reporting can better remind users to 
use the correct speed configuration instead of directly using the 
default configuration.

Re: [PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Jagan Teki]

On Tue, Feb 22, 2022 at 7:01 AM Peter Geis <pgwipeout@gmail.com> wrote:
>
> The reset handler for rk3568 is missing its private data. This leads to
> an abort when a reset is triggered.
>
> Add the missing dev_set_priv to the rk3568 clk driver.
>
> Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # radxa-cm3

Re: [PATCH v1 09/11] rockchip: move dwc3 config to chip specific handler

[Jagan Teki]

On Tue, Feb 22, 2022 at 7:03 AM Peter Geis <pgwipeout@gmail.com> wrote:
>
> The dwc3 code in the mach-rockchip board file is specific to the rk3399.
> Move it to the rk3399 chip specific code.

Though it is rk3399, there is no new SoC that requires OTG as of now
even if needed it is easy to support here instead of moving this
redundant on each board file.
https://patchwork.ozlabs.org/project/uboot/patch/20230226132234.31949-2-abbaraju.manojsai@amarulasolutions.com/

So, please don't move.

Jagan.