[PATCH V2 0/4] Anbernic RGxx3 Bootloader Fixes

From: Chris Morgan macromorgan@hotmail.com
Update the Anbernic RGxx3 "device" to use upstream device-trees, add logic to detect a different vdd_cpu regulator, and implement a fix to allow the panel auto-detection to run when using mainline A-TF.
Note that *Linux* still cannot use mainline A-TF because of the missing SCMI clock and reset functionality, but this patch series at least ensures that this board can boot once Linux is ready.
Chris Morgan (4): board: rockchip: Convert Anbernic RGxx3 to OF_UPSTREAM board: rockchip: Add vdd_cpu reg fixup for RGXX3 Series board: rockchip: Remove ARM SCMI Support from RGxx3 board: rockchip: Enable PD_VO before driver access
.../dts/rk3566-anbernic-rg353p-u-boot.dtsi | 34 ++ .../arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi | 52 --- arch/arm/dts/rk3566-anbernic-rgxx3.dts | 28 -- arch/arm/mach-rockchip/rk3568/rk3568.c | 6 + board/anbernic/rgxx3_rk3566/MAINTAINERS | 4 +- board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 359 +++++++++++++++--- configs/anbernic-rgxx3-rk3566_defconfig | 12 +- 7 files changed, 347 insertions(+), 148 deletions(-) create mode 100644 arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3.dts

From: Chris Morgan macromorgan@hotmail.com
Refactor the board detection logic (again) to make it compatible with the upstream device-trees, and switch to OF_UPSTREAM.
Now the device boots with the device-tree for the 353P, and then loads the correct device tree (of 10) in the later stages of SPL.
Signed-off-by: Chris Morgan macromorgan@hotmail.com --- .../dts/rk3566-anbernic-rg353p-u-boot.dtsi | 34 +++ .../arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi | 52 ----- arch/arm/dts/rk3566-anbernic-rgxx3.dts | 28 --- board/anbernic/rgxx3_rk3566/MAINTAINERS | 4 +- board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 214 +++++++++++++----- configs/anbernic-rgxx3-rk3566_defconfig | 10 +- 6 files changed, 195 insertions(+), 147 deletions(-) create mode 100644 arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3.dts
diff --git a/arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi b/arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi new file mode 100644 index 00000000000..fa3fbe6c810 --- /dev/null +++ b/arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) + +#include "rk356x-u-boot.dtsi" + +/ { + chosen { + u-boot,spl-boot-order = &sdmmc0, &sdhci; + }; + + /* + * Adding fixed regulator to work around driver regulator + * requirements. Note that the correct regulator is on by + * default at boot and that saradc regulator gets corrected + * when proper device-tree is loaded. + */ + vcc_1v8_dummy: vcc-1v8-dummy { + bootph-pre-ram; + bootph-some-ram; + compatible = "regulator-fixed"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-name = "vcc_1v8_dummy"; + status = "okay"; + }; +}; + +&saradc { + bootph-pre-ram; + bootph-some-ram; + vref-supply = <&vcc_1v8_dummy>; + status = "okay"; +}; diff --git a/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi b/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi deleted file mode 100644 index c7e849816a6..00000000000 --- a/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi +++ /dev/null @@ -1,52 +0,0 @@ -// SPDX-License-Identifier: (GPL-2.0+ OR MIT) - -#include "rk356x-u-boot.dtsi" - -/ { - chosen { - u-boot,spl-boot-order = &sdmmc0, &sdhci; - }; -}; - -&dsi_dphy0 { - status = "okay"; -}; - -&dsi0 { - status = "okay"; -}; - -&i2c2 { - pinctrl-0 = <&i2c2m1_xfer>; - pinctrl-names = "default"; - status = "okay"; -}; - -&pmucru { - assigned-clocks = <&pmucru SCLK_32K_IOE>; - assigned-clock-parents = <&pmucru CLK_RTC_32K>; -}; - -/* - * We don't need the clocks, but if they are present they may cause - * probing to fail so we remove them for U-Boot. - */ -&rk817 { - /delete-property/ assigned-clocks; - /delete-property/ assigned-clock-parents; - /delete-property/ clocks; - /delete-property/ clock-names; -}; - -&sdhci { - pinctrl-0 = <&emmc_bus8>, <&emmc_clk>, <&emmc_cmd>, - <&emmc_datastrobe>, <&emmc_rstnout>; - pinctrl-names = "default"; - bus-width = <8>; - max-frequency = <200000000>; - mmc-hs200-1_8v; - non-removable; - vmmc-supply = <&vcc_3v3>; - vqmmc-supply = <&vcc_1v8>; - status = "okay"; -}; diff --git a/arch/arm/dts/rk3566-anbernic-rgxx3.dts b/arch/arm/dts/rk3566-anbernic-rgxx3.dts deleted file mode 100644 index c393c8d07af..00000000000 --- a/arch/arm/dts/rk3566-anbernic-rgxx3.dts +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-License-Identifier: (GPL-2.0+ OR MIT) - -/dts-v1/; - -#include "rk3566-anbernic-rgxx3.dtsi" - -/ { - -/* - * Note this is a pseudo-model that doesn't exist in mainline Linux. - * This model is used for all RGXX3 devices and the board.c file will - * set the correct dtb name for loading mainline Linux automatically. - */ - model = "RGXX3"; - compatible = "anbernic,rg-arc-d", "anbernic,rg-arc-s", - "anbernic,rg353m", "anbernic,rg353p", - "anbernic,rg353ps", "anbernic,rg353v", - "anbernic,rg353vs", "anbernic,rg503", - "powkiddy,rgb10max3", "powkiddy,rgb30", - "powkiddy,rk2023", "rockchip,rk3566"; -}; - -&cru { - assigned-clocks = <&pmucru CLK_RTC_32K>, <&cru PLL_GPLL>, - <&pmucru PLL_PPLL>, <&cru PLL_VPLL>; - assigned-clock-rates = <32768>, <1200000000>, - <200000000>, <241500000>; -}; diff --git a/board/anbernic/rgxx3_rk3566/MAINTAINERS b/board/anbernic/rgxx3_rk3566/MAINTAINERS index 7970e5a4aad..75a1e8076ca 100644 --- a/board/anbernic/rgxx3_rk3566/MAINTAINERS +++ b/board/anbernic/rgxx3_rk3566/MAINTAINERS @@ -4,6 +4,4 @@ S: Maintained F: board/anbernic/rgxx3_rk3566 F: include/configs/anbernic-rgxx3-rk3566.h F: configs/anbernic-rgxx3-rk3566_defconfig -F: arch/arm/dts/rk3566-anbernic-rgxx3.dts -F: arch/arm/dts/rk3566-anbernic-rgxx3.dtsi -F: arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi +F: arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 5c57b902d14..224019f9ba3 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -11,6 +11,7 @@ #include <dm/lists.h> #include <env.h> #include <fdt_support.h> +#include <i2c.h> #include <linux/delay.h> #include <mipi_dsi.h> #include <mmc.h> @@ -19,6 +20,8 @@ #include <stdlib.h> #include <video_bridge.h>
+DECLARE_GLOBAL_DATA_PTR; + #define GPIO0_BASE 0xfdd60000 #define GPIO4_BASE 0xfe770000 #define GPIO_SWPORT_DR_L 0x0000 @@ -40,10 +43,11 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel; + const bool uart_con; };
enum rgxx3_device_id { - RG353M, + RG353M = 1, RG353P, RG353V, RG503, @@ -61,45 +65,51 @@ static const struct rg3xx_model rg3xx_model_details[] = { [RG353M] = { .adc_value = 517, /* Observed average from device */ .board = "rk3566-anbernic-rg353m", - .board_name = "RG353M", + .board_name = "Anbernic RG353M", /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1, + .uart_con = 1, }, [RG353P] = { .adc_value = 860, /* Documented value of 860 */ .board = "rk3566-anbernic-rg353p", - .board_name = "RG353P", + .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1, + .uart_con = 1, }, [RG353V] = { .adc_value = 695, /* Observed average from device */ .board = "rk3566-anbernic-rg353v", - .board_name = "RG353V", + .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1, + .uart_con = 1, }, [RG503] = { .adc_value = 1023, /* Observed average from device */ .board = "rk3566-anbernic-rg503", - .board_name = "RG503", + .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0, + .uart_con = 1, }, [RGB30] = { .adc_value = 383, /* Gathered from second hand information */ .board = "rk3566-powkiddy-rgb30", - .board_name = "RGB30", + .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0, + .uart_con = 0, }, [RK2023] = { .adc_value = 635, /* Observed average from device */ .board = "rk3566-powkiddy-rk2023", - .board_name = "RK2023", + .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0, + .uart_con = 0, }, [RGARCD] = { .adc_value = 183, /* Observed average from device */ @@ -107,6 +117,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0, + .uart_con = 1, }, [RGB10MAX3] = { .adc_value = 765, /* Observed average from device */ @@ -114,21 +125,24 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0, + .uart_con = 0, }, /* Devices with duplicate ADC value */ [RG353PS] = { .adc_value = 860, /* Observed average from device */ .board = "rk3566-anbernic-rg353ps", - .board_name = "RG353PS", + .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1, + .uart_con = 1, }, [RG353VS] = { .adc_value = 695, /* Gathered from second hand information */ .board = "rk3566-anbernic-rg353vs", - .board_name = "RG353VS", + .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1, + .uart_con = 1, }, [RGARCS] = { .adc_value = 183, /* Observed average from device */ @@ -136,6 +150,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0, + .uart_con = 1, }, };
@@ -164,7 +179,7 @@ static const struct rg353_panel rg353_panel_details[] = { void spl_board_init(void) { /* Set GPIO0_C5, GPIO0_C6, and GPIO0_C7 to output. */ - writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | \ + writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | (GPIO_C7 | GPIO_C6 | GPIO_C5), (GPIO0_BASE + GPIO_SWPORT_DDR_H)); /* Set GPIO0_C5 and GPIO_C6 to 0 and GPIO0_C7 to 1. */ @@ -174,16 +189,22 @@ void spl_board_init(void)
/* * Buzz the buzzer so the user knows something is going on. Make it - * optional in case PWM is disabled. + * optional in case PWM is disabled or if CONFIG_DM_PWM is not + * enabled. */ void __maybe_unused startup_buzz(void) { struct udevice *dev; int err;
- err = uclass_get_device(UCLASS_PWM, 0, &dev); + if (!IS_ENABLED(CONFIG_DM_PWM)) + return; + + /* Probe the PWM controller. */ + err = uclass_get_device_by_name(UCLASS_PWM, + "pwm@fe6e0010", &dev); if (err) - printf("pwm not found\n"); + return;
pwm_set_enable(dev, 0, 1); mdelay(200); @@ -245,6 +266,13 @@ U_BOOT_DRIVER(anbernic_rg353_panel) = { .plat_auto = sizeof(struct mipi_dsi_panel_plat), };
+/* + * The Anbernic 353 series shipped with 2 distinct displays requiring + * 2 distinct drivers, with no way for a user to know which panel is + * which. This function queries the DSI panel for the panel ID to + * determine which panel is present so the device-tree can be corrected + * automatically. + */ int rgxx3_detect_display(void) { struct udevice *dev; @@ -333,17 +361,10 @@ int rgxx3_detect_display(void) return 0; }
-/* Detect which Anbernic RGXX3 device we are using so as to load the - * correct devicetree for Linux. Set an environment variable once - * found. The detection depends on the value of ADC channel 1, the - * presence of an eMMC on mmc0, and querying the DSI panel. - */ -int rgxx3_detect_device(void) +int rgxx3_read_board_id(void) { u32 adc_info; - int ret, i; - int board_id = -ENXIO; - struct mmc *mmc; + int ret;
ret = adc_channel_single_shot("saradc@fe720000", 1, &adc_info); if (ret) { @@ -357,16 +378,32 @@ int rgxx3_detect_device(void) * design calls for no more than a 1% variance on the * resistor, so assume a +- value of 15 should be enough. */ - for (i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) { + for (int i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) { u32 adc_min = rg3xx_model_details[i].adc_value - 15; u32 adc_max = rg3xx_model_details[i].adc_value + 15;
- if (adc_min < adc_info && adc_max > adc_info) { - board_id = i; - break; - } + if (adc_min < adc_info && adc_max > adc_info) + return i; }
+ return -ENODEV; +} + +/* Detect which Anbernic RGXX3 device we are using so as to load the + * correct devicetree for Linux. Set an environment variable once + * found. The detection depends on the value of ADC channel 1 and the + * presence of an eMMC on mmc0. + */ +int rgxx3_detect_device(void) +{ + int ret; + int board_id; + struct mmc *mmc; + + board_id = rgxx3_read_board_id(); + if (board_id < 0) + return board_id; + /* * Try to access the eMMC on an RG353V, RG353P, or RG Arc D. * If it's missing, it's an RG353VS, RG353PS, or RG Arc S. @@ -387,67 +424,87 @@ int rgxx3_detect_device(void) } }
- if (board_id < 0) - return board_id; + return board_id; +}
- env_set("board", rg3xx_model_details[board_id].board); - env_set("board_name", - rg3xx_model_details[board_id].board_name); - env_set("fdtfile", rg3xx_model_details[board_id].fdtfile); +/* + * Check the loaded device tree to set the correct gd->board_type. + * Disable the console if the board doesn't support a console. + */ +int set_gd_value(void) +{ + const char *model;
- /* Skip panel detection for when it is not needed. */ - if (!rg3xx_model_details[board_id].detect_panel) - return 0; + model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
- /* Warn but don't fail for errors in auto-detection of the panel. */ - ret = rgxx3_detect_display(); - if (ret) - printf("Failed to detect panel type\n"); + for (int i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) { + if (strcmp(rg3xx_model_details[i].board_name, model) == 0) { + gd->board_type = i; + if (!rg3xx_model_details[i].uart_con) + gd->flags |= GD_FLG_SILENT | + GD_FLG_DISABLE_CONSOLE; + return 0; + } + }
- return 0; + return -ENODEV; }
int rk_board_late_init(void) { int ret;
- ret = rgxx3_detect_device(); + ret = set_gd_value(); if (ret) { - printf("Unable to detect device type: %d\n", ret); - return ret; + printf("Unable to auto-detect device\n"); + goto end; }
+ /* + * Change the model number on the RG353M since it uses the same + * tree as the RG353P. + */ + if (gd->board_type == RG353P) { + ret = rgxx3_read_board_id(); + if (ret > 0) + gd->board_type = ret; + } + + env_set("board", rg3xx_model_details[gd->board_type].board); + env_set("board_name", + rg3xx_model_details[gd->board_type].board_name); + env_set("fdtfile", rg3xx_model_details[gd->board_type].fdtfile); + + /* + * Skip panel detection if not needed. Warn but don't fail for + * errors in auto-detection of the panel. + */ + if (rg3xx_model_details[gd->board_type].detect_panel) { + ret = rgxx3_detect_display(); + if (ret) + printf("Failed to detect panel type\n"); + } + +end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6, (GPIO0_BASE + GPIO_SWPORT_DR_H));
- if (IS_ENABLED(CONFIG_DM_PWM)) - startup_buzz(); + startup_buzz();
return 0; }
-int ft_board_setup(void *blob, struct bd_info *bd) +int rgxx3_panel_fixup(void *blob) { const struct rg353_panel *panel = NULL; - int node, ret, i; + int node, ret; char *env;
- /* No fixups necessary for the RG503 */ - env = env_get("board_name"); - if (env && (!strcmp(env, rg3xx_model_details[RG503].board_name))) - return 0; - - /* Change the model name of the RG353M */ - if (env && (!strcmp(env, rg3xx_model_details[RG353M].board_name))) - fdt_setprop(blob, 0, "model", - rg3xx_model_details[RG353M].board_name, - sizeof(rg3xx_model_details[RG353M].board_name)); - env = env_get("panel"); if (!env) { printf("Can't get panel env\n"); - return 0; + return -EINVAL; }
/* @@ -469,7 +526,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) return 0;
/* Panels don't match, search by first compatible value. */ - for (i = 0; i < ARRAY_SIZE(rg353_panel_details); i++) { + for (int i = 0; i < ARRAY_SIZE(rg353_panel_details); i++) { if (!strcmp(env, rg353_panel_details[i].panel_compat[0])) { panel = &rg353_panel_details[i]; break; @@ -489,3 +546,38 @@ int ft_board_setup(void *blob, struct bd_info *bd)
return 0; } + +int ft_board_setup(void *blob, struct bd_info *bd) +{ + int ret; + + if (gd->board_type == RG353M) + fdt_setprop(blob, 0, "model", + rg3xx_model_details[RG353M].board_name, + sizeof(rg3xx_model_details[RG353M].board_name)); + + if (rg3xx_model_details[gd->board_type].detect_panel) { + ret = rgxx3_panel_fixup(blob); + if (ret) + printf("Unable to update panel compat\n"); + } + + return 0; +} + +int board_fit_config_name_match(const char *name) +{ + int ret; + + if (gd->board_type == 0) { + ret = rgxx3_detect_device(); + if (ret < 0) + return ret; + gd->board_type = ret; + } + + if (strcmp(name, rg3xx_model_details[gd->board_type].fdtfile) == 0) + return 0; + + return -ENXIO; +} diff --git a/configs/anbernic-rgxx3-rk3566_defconfig b/configs/anbernic-rgxx3-rk3566_defconfig index a03509bf467..f5e5470df8c 100644 --- a/configs/anbernic-rgxx3-rk3566_defconfig +++ b/configs/anbernic-rgxx3-rk3566_defconfig @@ -3,7 +3,7 @@ CONFIG_SKIP_LOWLEVEL_INIT=y CONFIG_COUNTER_FREQUENCY=24000000 CONFIG_ARCH_ROCKCHIP=y CONFIG_SPL_GPIO=y -CONFIG_DEFAULT_DEVICE_TREE="rk3566-anbernic-rgxx3" +CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3566-anbernic-rg353p" CONFIG_ROCKCHIP_RK3568=y CONFIG_ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON=y CONFIG_SPL_SERIAL=y @@ -19,8 +19,10 @@ CONFIG_SPL_LOAD_FIT=y CONFIG_LEGACY_IMAGE_FORMAT=y CONFIG_OF_BOARD_SETUP=y CONFIG_OF_STDOUT_VIA_ALIAS=y -CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-anbernic-rgxx3.dtb" +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-anbernic-rg353p.dtb" +CONFIG_DISABLE_CONSOLE=y # CONFIG_CONSOLE_MUX is not set +CONFIG_BOARD_TYPES=y # CONFIG_DISPLAY_CPUINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_BOARD_RNG_SEED=y @@ -38,13 +40,14 @@ CONFIG_CMD_MMC=y # CONFIG_SPL_DOS_PARTITION is not set CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIVE=y -# CONFIG_OF_UPSTREAM is not set +CONFIG_OF_LIST="rockchip/rk3566-anbernic-rg353p rockchip/rk3566-anbernic-rg353v rockchip/rk3566-anbernic-rg503 rockchip/rk3566-anbernic-rg-arc-d rockchip/rk3566-anbernic-rg353ps rockchip/rk3566-anbernic-rg353vs rockchip/rk3566-anbernic-rg-arc-s rockchip/rk3566-powkiddy-rgb30 rockchip/rk3566-powkiddy-rk2023 rockchip/rk3566-powkiddy-rgb10max3" CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y # CONFIG_NET is not set CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_SPL_REGMAP=y CONFIG_SPL_SYSCON=y +CONFIG_SPL_ADC=y CONFIG_SPL_CLK=y CONFIG_ARM_SMCCC_FEATURES=y CONFIG_SCMI_FIRMWARE=y @@ -70,6 +73,7 @@ CONFIG_SPL_RAM=y # CONFIG_RAM_ROCKCHIP_DEBUG is not set # CONFIG_RNG_SMCCC_TRNG is not set CONFIG_BAUDRATE=1500000 +# CONFIG_REQUIRE_SERIAL_CONSOLE is not set CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550_MEM32=y CONFIG_SYSRESET=y

On 2024/9/19 22:00, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Refactor the board detection logic (again) to make it compatible with the upstream device-trees, and switch to OF_UPSTREAM.
Now the device boots with the device-tree for the 353P, and then loads the correct device tree (of 10) in the later stages of SPL.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
.../dts/rk3566-anbernic-rg353p-u-boot.dtsi | 34 +++ .../arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi | 52 ----- arch/arm/dts/rk3566-anbernic-rgxx3.dts | 28 --- board/anbernic/rgxx3_rk3566/MAINTAINERS | 4 +- board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 214 +++++++++++++----- configs/anbernic-rgxx3-rk3566_defconfig | 10 +- 6 files changed, 195 insertions(+), 147 deletions(-) create mode 100644 arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3.dts
diff --git a/arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi b/arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi new file mode 100644 index 00000000000..fa3fbe6c810 --- /dev/null +++ b/arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+#include "rk356x-u-boot.dtsi"
+/ {
- chosen {
u-boot,spl-boot-order = &sdmmc0, &sdhci;
- };
- /*
* Adding fixed regulator to work around driver regulator
* requirements. Note that the correct regulator is on by
* default at boot and that saradc regulator gets corrected
* when proper device-tree is loaded.
*/
- vcc_1v8_dummy: vcc-1v8-dummy {
bootph-pre-ram;
bootph-some-ram;
compatible = "regulator-fixed";
regulator-always-on;
regulator-boot-on;
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-name = "vcc_1v8_dummy";
status = "okay";
- };
+};
+&saradc {
- bootph-pre-ram;
- bootph-some-ram;
- vref-supply = <&vcc_1v8_dummy>;
- status = "okay";
+}; diff --git a/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi b/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi deleted file mode 100644 index c7e849816a6..00000000000 --- a/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi +++ /dev/null @@ -1,52 +0,0 @@ -// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
-#include "rk356x-u-boot.dtsi"
-/ {
- chosen {
u-boot,spl-boot-order = &sdmmc0, &sdhci;
- };
-};
-&dsi_dphy0 {
- status = "okay";
-};
-&dsi0 {
- status = "okay";
-};
-&i2c2 {
- pinctrl-0 = <&i2c2m1_xfer>;
- pinctrl-names = "default";
- status = "okay";
-};
-&pmucru {
- assigned-clocks = <&pmucru SCLK_32K_IOE>;
- assigned-clock-parents = <&pmucru CLK_RTC_32K>;
-};
-/*
- We don't need the clocks, but if they are present they may cause
- probing to fail so we remove them for U-Boot.
- */
-&rk817 {
- /delete-property/ assigned-clocks;
- /delete-property/ assigned-clock-parents;
- /delete-property/ clocks;
- /delete-property/ clock-names;
-};
-&sdhci {
- pinctrl-0 = <&emmc_bus8>, <&emmc_clk>, <&emmc_cmd>,
<&emmc_datastrobe>, <&emmc_rstnout>;
- pinctrl-names = "default";
- bus-width = <8>;
- max-frequency = <200000000>;
- mmc-hs200-1_8v;
- non-removable;
- vmmc-supply = <&vcc_3v3>;
- vqmmc-supply = <&vcc_1v8>;
- status = "okay";
-}; diff --git a/arch/arm/dts/rk3566-anbernic-rgxx3.dts b/arch/arm/dts/rk3566-anbernic-rgxx3.dts deleted file mode 100644 index c393c8d07af..00000000000 --- a/arch/arm/dts/rk3566-anbernic-rgxx3.dts +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
-/dts-v1/;
-#include "rk3566-anbernic-rgxx3.dtsi"
-/ {
-/*
- Note this is a pseudo-model that doesn't exist in mainline Linux.
- This model is used for all RGXX3 devices and the board.c file will
- set the correct dtb name for loading mainline Linux automatically.
- */
- model = "RGXX3";
- compatible = "anbernic,rg-arc-d", "anbernic,rg-arc-s",
"anbernic,rg353m", "anbernic,rg353p",
"anbernic,rg353ps", "anbernic,rg353v",
"anbernic,rg353vs", "anbernic,rg503",
"powkiddy,rgb10max3", "powkiddy,rgb30",
"powkiddy,rk2023", "rockchip,rk3566";
-};
-&cru {
- assigned-clocks = <&pmucru CLK_RTC_32K>, <&cru PLL_GPLL>,
<&pmucru PLL_PPLL>, <&cru PLL_VPLL>;
- assigned-clock-rates = <32768>, <1200000000>,
<200000000>, <241500000>;
-}; diff --git a/board/anbernic/rgxx3_rk3566/MAINTAINERS b/board/anbernic/rgxx3_rk3566/MAINTAINERS index 7970e5a4aad..75a1e8076ca 100644 --- a/board/anbernic/rgxx3_rk3566/MAINTAINERS +++ b/board/anbernic/rgxx3_rk3566/MAINTAINERS @@ -4,6 +4,4 @@ S: Maintained F: board/anbernic/rgxx3_rk3566 F: include/configs/anbernic-rgxx3-rk3566.h F: configs/anbernic-rgxx3-rk3566_defconfig -F: arch/arm/dts/rk3566-anbernic-rgxx3.dts -F: arch/arm/dts/rk3566-anbernic-rgxx3.dtsi -F: arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi +F: arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 5c57b902d14..224019f9ba3 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -11,6 +11,7 @@ #include <dm/lists.h> #include <env.h> #include <fdt_support.h> +#include <i2c.h> #include <linux/delay.h> #include <mipi_dsi.h> #include <mmc.h> @@ -19,6 +20,8 @@ #include <stdlib.h> #include <video_bridge.h>
+DECLARE_GLOBAL_DATA_PTR;
- #define GPIO0_BASE 0xfdd60000 #define GPIO4_BASE 0xfe770000 #define GPIO_SWPORT_DR_L 0x0000
@@ -40,10 +43,11 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel;
const bool uart_con; };
enum rgxx3_device_id {
- RG353M,
- RG353M = 1, RG353P, RG353V, RG503,
@@ -61,45 +65,51 @@ static const struct rg3xx_model rg3xx_model_details[] = { [RG353M] = { .adc_value = 517, /* Observed average from device */ .board = "rk3566-anbernic-rg353m",
.board_name = "RG353M",
/* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,.board_name = "Anbernic RG353M",
}, [RG353P] = { .adc_value = 860, /* Documented value of 860 */ .board = "rk3566-anbernic-rg353p",.uart_con = 1,
.board_name = "RG353P",
.fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,.board_name = "Anbernic RG353P",
}, [RG353V] = { .adc_value = 695, /* Observed average from device */ .board = "rk3566-anbernic-rg353v",.uart_con = 1,
.board_name = "RG353V",
.fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1,.board_name = "Anbernic RG353V",
}, [RG503] = { .adc_value = 1023, /* Observed average from device */ .board = "rk3566-anbernic-rg503",.uart_con = 1,
.board_name = "RG503",
.fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0,.board_name = "Anbernic RG503",
}, [RGB30] = { .adc_value = 383, /* Gathered from second hand information */ .board = "rk3566-powkiddy-rgb30",.uart_con = 1,
.board_name = "RGB30",
.fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0,.board_name = "Powkiddy RGB30",
}, [RK2023] = { .adc_value = 635, /* Observed average from device */ .board = "rk3566-powkiddy-rk2023",.uart_con = 0,
.board_name = "RK2023",
.fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0,.board_name = "Powkiddy RK2023",
}, [RGARCD] = { .adc_value = 183, /* Observed average from device */.uart_con = 0,
@@ -107,6 +117,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0,
}, [RGB10MAX3] = { .adc_value = 765, /* Observed average from device */.uart_con = 1,
@@ -114,21 +125,24 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0,
}, /* Devices with duplicate ADC value */ [RG353PS] = { .adc_value = 860, /* Observed average from device */ .board = "rk3566-anbernic-rg353ps",.uart_con = 0,
.board_name = "RG353PS",
.fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1,.board_name = "Anbernic RG353PS",
}, [RG353VS] = { .adc_value = 695, /* Gathered from second hand information */ .board = "rk3566-anbernic-rg353vs",.uart_con = 1,
.board_name = "RG353VS",
.fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1,.board_name = "Anbernic RG353VS",
}, [RGARCS] = { .adc_value = 183, /* Observed average from device */.uart_con = 1,
@@ -136,6 +150,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0,
}, };.uart_con = 1,
@@ -164,7 +179,7 @@ static const struct rg353_panel rg353_panel_details[] = { void spl_board_init(void) { /* Set GPIO0_C5, GPIO0_C6, and GPIO0_C7 to output. */
- writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | \
- writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | (GPIO_C7 | GPIO_C6 | GPIO_C5), (GPIO0_BASE + GPIO_SWPORT_DDR_H)); /* Set GPIO0_C5 and GPIO_C6 to 0 and GPIO0_C7 to 1. */
@@ -174,16 +189,22 @@ void spl_board_init(void)
/*
- Buzz the buzzer so the user knows something is going on. Make it
- optional in case PWM is disabled.
- optional in case PWM is disabled or if CONFIG_DM_PWM is not
*/ void __maybe_unused startup_buzz(void) { struct udevice *dev; int err;
- enabled.
- err = uclass_get_device(UCLASS_PWM, 0, &dev);
- if (!IS_ENABLED(CONFIG_DM_PWM))
return;
- /* Probe the PWM controller. */
- err = uclass_get_device_by_name(UCLASS_PWM,
if (err)"pwm@fe6e0010", &dev);
printf("pwm not found\n");
return;
pwm_set_enable(dev, 0, 1); mdelay(200);
@@ -245,6 +266,13 @@ U_BOOT_DRIVER(anbernic_rg353_panel) = { .plat_auto = sizeof(struct mipi_dsi_panel_plat), };
+/*
- The Anbernic 353 series shipped with 2 distinct displays requiring
- 2 distinct drivers, with no way for a user to know which panel is
- which. This function queries the DSI panel for the panel ID to
- determine which panel is present so the device-tree can be corrected
- automatically.
- */ int rgxx3_detect_display(void) { struct udevice *dev;
@@ -333,17 +361,10 @@ int rgxx3_detect_display(void) return 0; }
-/* Detect which Anbernic RGXX3 device we are using so as to load the
- correct devicetree for Linux. Set an environment variable once
- found. The detection depends on the value of ADC channel 1, the
- presence of an eMMC on mmc0, and querying the DSI panel.
- */
-int rgxx3_detect_device(void) +int rgxx3_read_board_id(void) { u32 adc_info;
- int ret, i;
- int board_id = -ENXIO;
- struct mmc *mmc;
int ret;
ret = adc_channel_single_shot("saradc@fe720000", 1, &adc_info); if (ret) {
@@ -357,16 +378,32 @@ int rgxx3_detect_device(void) * design calls for no more than a 1% variance on the * resistor, so assume a +- value of 15 should be enough. */
- for (i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) {
- for (int i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) { u32 adc_min = rg3xx_model_details[i].adc_value - 15; u32 adc_max = rg3xx_model_details[i].adc_value + 15;
if (adc_min < adc_info && adc_max > adc_info) {
board_id = i;
break;
}
if (adc_min < adc_info && adc_max > adc_info)
return i;
}
return -ENODEV;
+}
+/* Detect which Anbernic RGXX3 device we are using so as to load the
- correct devicetree for Linux. Set an environment variable once
- found. The detection depends on the value of ADC channel 1 and the
- presence of an eMMC on mmc0.
- */
+int rgxx3_detect_device(void) +{
- int ret;
- int board_id;
- struct mmc *mmc;
- board_id = rgxx3_read_board_id();
- if (board_id < 0)
return board_id;
- /*
- Try to access the eMMC on an RG353V, RG353P, or RG Arc D.
- If it's missing, it's an RG353VS, RG353PS, or RG Arc S.
@@ -387,67 +424,87 @@ int rgxx3_detect_device(void) } }
- if (board_id < 0)
return board_id;
- return board_id;
+}
- env_set("board", rg3xx_model_details[board_id].board);
- env_set("board_name",
rg3xx_model_details[board_id].board_name);
- env_set("fdtfile", rg3xx_model_details[board_id].fdtfile);
+/*
- Check the loaded device tree to set the correct gd->board_type.
- Disable the console if the board doesn't support a console.
- */
+int set_gd_value(void) +{
- const char *model;
- /* Skip panel detection for when it is not needed. */
- if (!rg3xx_model_details[board_id].detect_panel)
return 0;
- model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
- /* Warn but don't fail for errors in auto-detection of the panel. */
- ret = rgxx3_detect_display();
- if (ret)
printf("Failed to detect panel type\n");
- for (int i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) {
if (strcmp(rg3xx_model_details[i].board_name, model) == 0) {
gd->board_type = i;
if (!rg3xx_model_details[i].uart_con)
gd->flags |= GD_FLG_SILENT |
GD_FLG_DISABLE_CONSOLE;
return 0;
}
- }
- return 0;
return -ENODEV; }
int rk_board_late_init(void) { int ret;
- ret = rgxx3_detect_device();
- ret = set_gd_value(); if (ret) {
printf("Unable to detect device type: %d\n", ret);
return ret;
printf("Unable to auto-detect device\n");
goto end;
}
/*
* Change the model number on the RG353M since it uses the same
* tree as the RG353P.
*/
if (gd->board_type == RG353P) {
ret = rgxx3_read_board_id();
if (ret > 0)
gd->board_type = ret;
}
env_set("board", rg3xx_model_details[gd->board_type].board);
env_set("board_name",
rg3xx_model_details[gd->board_type].board_name);
env_set("fdtfile", rg3xx_model_details[gd->board_type].fdtfile);
/*
* Skip panel detection if not needed. Warn but don't fail for
* errors in auto-detection of the panel.
*/
if (rg3xx_model_details[gd->board_type].detect_panel) {
ret = rgxx3_detect_display();
if (ret)
printf("Failed to detect panel type\n");
}
+end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6, (GPIO0_BASE + GPIO_SWPORT_DR_H));
- if (IS_ENABLED(CONFIG_DM_PWM))
startup_buzz();
startup_buzz();
return 0; }
-int ft_board_setup(void *blob, struct bd_info *bd) +int rgxx3_panel_fixup(void *blob) { const struct rg353_panel *panel = NULL;
- int node, ret, i;
- int node, ret; char *env;
- /* No fixups necessary for the RG503 */
- env = env_get("board_name");
- if (env && (!strcmp(env, rg3xx_model_details[RG503].board_name)))
return 0;
- /* Change the model name of the RG353M */
- if (env && (!strcmp(env, rg3xx_model_details[RG353M].board_name)))
fdt_setprop(blob, 0, "model",
rg3xx_model_details[RG353M].board_name,
sizeof(rg3xx_model_details[RG353M].board_name));
- env = env_get("panel"); if (!env) { printf("Can't get panel env\n");
return 0;
return -EINVAL;
}
/*
@@ -469,7 +526,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) return 0;
/* Panels don't match, search by first compatible value. */
- for (i = 0; i < ARRAY_SIZE(rg353_panel_details); i++) {
- for (int i = 0; i < ARRAY_SIZE(rg353_panel_details); i++) { if (!strcmp(env, rg353_panel_details[i].panel_compat[0])) { panel = &rg353_panel_details[i]; break;
@@ -489,3 +546,38 @@ int ft_board_setup(void *blob, struct bd_info *bd)
return 0; }
+int ft_board_setup(void *blob, struct bd_info *bd) +{
- int ret;
- if (gd->board_type == RG353M)
fdt_setprop(blob, 0, "model",
rg3xx_model_details[RG353M].board_name,
sizeof(rg3xx_model_details[RG353M].board_name));
- if (rg3xx_model_details[gd->board_type].detect_panel) {
ret = rgxx3_panel_fixup(blob);
if (ret)
printf("Unable to update panel compat\n");
- }
- return 0;
+}
+int board_fit_config_name_match(const char *name) +{
- int ret;
- if (gd->board_type == 0) {
ret = rgxx3_detect_device();
if (ret < 0)
return ret;
gd->board_type = ret;
- }
- if (strcmp(name, rg3xx_model_details[gd->board_type].fdtfile) == 0)
return 0;
- return -ENXIO;
+} diff --git a/configs/anbernic-rgxx3-rk3566_defconfig b/configs/anbernic-rgxx3-rk3566_defconfig index a03509bf467..f5e5470df8c 100644 --- a/configs/anbernic-rgxx3-rk3566_defconfig +++ b/configs/anbernic-rgxx3-rk3566_defconfig @@ -3,7 +3,7 @@ CONFIG_SKIP_LOWLEVEL_INIT=y CONFIG_COUNTER_FREQUENCY=24000000 CONFIG_ARCH_ROCKCHIP=y CONFIG_SPL_GPIO=y -CONFIG_DEFAULT_DEVICE_TREE="rk3566-anbernic-rgxx3" +CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3566-anbernic-rg353p" CONFIG_ROCKCHIP_RK3568=y CONFIG_ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON=y CONFIG_SPL_SERIAL=y @@ -19,8 +19,10 @@ CONFIG_SPL_LOAD_FIT=y CONFIG_LEGACY_IMAGE_FORMAT=y CONFIG_OF_BOARD_SETUP=y CONFIG_OF_STDOUT_VIA_ALIAS=y -CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-anbernic-rgxx3.dtb" +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-anbernic-rg353p.dtb" +CONFIG_DISABLE_CONSOLE=y # CONFIG_CONSOLE_MUX is not set +CONFIG_BOARD_TYPES=y # CONFIG_DISPLAY_CPUINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_BOARD_RNG_SEED=y @@ -38,13 +40,14 @@ CONFIG_CMD_MMC=y # CONFIG_SPL_DOS_PARTITION is not set CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIVE=y -# CONFIG_OF_UPSTREAM is not set +CONFIG_OF_LIST="rockchip/rk3566-anbernic-rg353p rockchip/rk3566-anbernic-rg353v rockchip/rk3566-anbernic-rg503 rockchip/rk3566-anbernic-rg-arc-d rockchip/rk3566-anbernic-rg353ps rockchip/rk3566-anbernic-rg353vs rockchip/rk3566-anbernic-rg-arc-s rockchip/rk3566-powkiddy-rgb30 rockchip/rk3566-powkiddy-rk2023 rockchip/rk3566-powkiddy-rgb10max3" CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y # CONFIG_NET is not set CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_SPL_REGMAP=y CONFIG_SPL_SYSCON=y +CONFIG_SPL_ADC=y CONFIG_SPL_CLK=y CONFIG_ARM_SMCCC_FEATURES=y CONFIG_SCMI_FIRMWARE=y @@ -70,6 +73,7 @@ CONFIG_SPL_RAM=y # CONFIG_RAM_ROCKCHIP_DEBUG is not set # CONFIG_RNG_SMCCC_TRNG is not set CONFIG_BAUDRATE=1500000 +# CONFIG_REQUIRE_SERIAL_CONSOLE is not set CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550_MEM32=y CONFIG_SYSRESET=y

From: Chris Morgan macromorgan@hotmail.com
Some of the Powkiddy devices switched to using a different vendor for the vdd_cpu regulator. Unfortunately the device does not have a new revision to denote this, so users have no way of knowing in advance.
Add code to detect if a device is present at addresses 0x1c or 0x40 on the i2c0 bus and update the devicetree if needed.
Signed-off-by: Chris Morgan macromorgan@hotmail.com --- board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 224019f9ba3..c1d1826fd14 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -43,6 +43,7 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel; + const bool detect_regulator; const bool uart_con; };
@@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1, + .detect_regulator = 0, .uart_con = 1, }, [RG353P] = { @@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1, + .detect_regulator = 0, .uart_con = 1, }, [RG353V] = { @@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1, + .detect_regulator = 0, .uart_con = 1, }, [RG503] = { @@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0, + .detect_regulator = 0, .uart_con = 1, }, [RGB30] = { @@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0, + .detect_regulator = 1, .uart_con = 0, }, [RK2023] = { @@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0, + .detect_regulator = 1, .uart_con = 0, }, [RGARCD] = { @@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0, + .detect_regulator = 0, .uart_con = 1, }, [RGB10MAX3] = { @@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0, + .detect_regulator = 1, .uart_con = 0, }, /* Devices with duplicate ADC value */ @@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1, + .detect_regulator = 0, .uart_con = 1, }, [RG353VS] = { @@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1, + .detect_regulator = 0, .uart_con = 1, }, [RGARCS] = { @@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0, + .detect_regulator = 0, .uart_con = 1, }, }; @@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = { }, };
+struct powkiddy_regulators { + const u8 addr; + const char *regulator_compat; +}; + +static const struct powkiddy_regulators regulator_details[] = { + { + .addr = 0x1c, + .regulator_compat = "tcs,tcs4525", + }, + { + .addr = 0x40, + .regulator_compat = "fcs,fan53555", + }, +}; + /* * Start LED very early so user knows device is on. Set color * to red. @@ -361,6 +389,44 @@ int rgxx3_detect_display(void) return 0; }
+/* + * Some of the Powkiddy devices switched the CPU regulator, but users + * are not able to determine this by looking at their hardware. + * Attempt to auto-detect this situation and fixup the device-tree. + */ +int rgxx3_detect_regulator(void) +{ + struct udevice *bus; + struct udevice *chip; + u8 val; + int ret; + + /* Get the correct i2c bus (i2c0). */ + ret = uclass_get_device_by_name(UCLASS_I2C, + "i2c@fdd40000", &bus); + if (ret) + return ret; + + /* + * Check for all vdd_cpu regulators and read an arbitrary + * register to confirm it's present. + */ + for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) { + ret = i2c_get_chip(bus, regulator_details[i].addr, + 1, &chip); + if (ret) + return ret; + + ret = dm_i2c_read(chip, 0, &val, 1); + if (!ret) { + env_set("vdd_cpu", regulator_details[i].regulator_compat); + break; + } + } + + return 0; +} + int rgxx3_read_board_id(void) { u32 adc_info; @@ -485,6 +551,16 @@ int rk_board_late_init(void) printf("Failed to detect panel type\n"); }
+ /* + * Skip vdd_cpu regulator detection if not needed. Warn but + * don't fail for errors in auto-detection of regulator. + */ + if (rg3xx_model_details[gd->board_type].detect_regulator) { + ret = rgxx3_detect_regulator(); + if (ret) + printf("Unable to detect vdd_cpu regulator\n"); + } + end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6, @@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob) return 0; }
+int rgxx3_regulator_fixup(void *blob) +{ + const struct powkiddy_regulators *vdd_cpu = NULL; + int node, ret, i; + char path[] = "/i2c@fdd40000/regulator@00"; + char name[] = "regulator@00"; + char *env; + + env = env_get("vdd_cpu"); + if (!env) { + printf("Can't get vdd_cpu env\n"); + return -EINVAL; + } + + /* + * Find the device we have in our tree, which may or may not + * be present. + */ + for (i = 0; i < ARRAY_SIZE(regulator_details); i++) { + sprintf(path, "/i2c@fdd40000/regulator@%02x", + regulator_details[i].addr); + node = fdt_path_offset(blob, path); + if (node > 0) + break; + + printf("Unable to find vdd_cpu\n"); + return -ENODEV; + } + + node = fdt_path_offset(blob, path); + if (!(node > 0)) { + printf("Can't find the vdd_cpu node\n"); + return -ENODEV; + } + + ret = fdt_node_check_compatible(blob, node, env); + if (ret < 0) + return -ENODEV; + + /* vdd_cpu regulators match, return 0. */ + if (!ret) + return 0; + + /* Regulators don't match, search by first compatible value. */ + for (i = 0; i < ARRAY_SIZE(regulator_details); i++) { + if (!strcmp(env, regulator_details[i].regulator_compat)) { + vdd_cpu = ®ulator_details[i]; + break; + } + } + + if (!vdd_cpu) { + printf("Unable to identify vdd_cpu by compat string\n"); + return -ENODEV; + } + + /* Set the compatible and reg with the auto-detected values */ + fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat); + fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr); + sprintf(name, "regulator@%02x", vdd_cpu->addr); + fdt_set_name(blob, node, name); + + return 0; +} + int ft_board_setup(void *blob, struct bd_info *bd) { int ret; @@ -562,6 +703,12 @@ int ft_board_setup(void *blob, struct bd_info *bd) printf("Unable to update panel compat\n"); }
+ if (rg3xx_model_details[gd->board_type].detect_regulator) { + ret = rgxx3_regulator_fixup(blob); + if (ret) + printf("Unable to update vdd_cpu compat\n"); + } + return 0; }

Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Some of the Powkiddy devices switched to using a different vendor for the vdd_cpu regulator. Unfortunately the device does not have a new revision to denote this, so users have no way of knowing in advance.
Add code to detect if a device is present at addresses 0x1c or 0x40 on the i2c0 bus and update the devicetree if needed.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 224019f9ba3..c1d1826fd14 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -43,6 +43,7 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel;
- const bool detect_regulator; const bool uart_con; };
@@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353P] = {.detect_regulator = 0,
@@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353V] = {.detect_regulator = 0,
@@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG503] = {.detect_regulator = 0,
@@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0,
.uart_con = 1, }, [RGB30] = {.detect_regulator = 0,
@@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0,
.uart_con = 0, }, [RK2023] = {.detect_regulator = 1,
@@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0,
.uart_con = 0, }, [RGARCD] = {.detect_regulator = 1,
@@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0,
.uart_con = 1, }, [RGB10MAX3] = {.detect_regulator = 0,
@@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0,
.uart_con = 0, }, /* Devices with duplicate ADC value */.detect_regulator = 1,
@@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353VS] = {.detect_regulator = 0,
@@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1,
.uart_con = 1, }, [RGARCS] = {.detect_regulator = 0,
@@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0,
.uart_con = 1, }, };.detect_regulator = 0,
@@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = { }, };
+struct powkiddy_regulators {
- const u8 addr;
- const char *regulator_compat;
+};
+static const struct powkiddy_regulators regulator_details[] = {
- {
.addr = 0x1c,
.regulator_compat = "tcs,tcs4525",
- },
- {
.addr = 0x40,
.regulator_compat = "fcs,fan53555",
- },
+};
- /*
- Start LED very early so user knows device is on. Set color
- to red.
@@ -361,6 +389,44 @@ int rgxx3_detect_display(void) return 0; }
+/*
- Some of the Powkiddy devices switched the CPU regulator, but users
- are not able to determine this by looking at their hardware.
- Attempt to auto-detect this situation and fixup the device-tree.
- */
+int rgxx3_detect_regulator(void) +{
- struct udevice *bus;
- struct udevice *chip;
- u8 val;
- int ret;
- /* Get the correct i2c bus (i2c0). */
- ret = uclass_get_device_by_name(UCLASS_I2C,
"i2c@fdd40000", &bus);
- if (ret)
return ret;
- /*
* Check for all vdd_cpu regulators and read an arbitrary
* register to confirm it's present.
*/
- for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) {
ret = i2c_get_chip(bus, regulator_details[i].addr,
1, &chip);
if (ret)
return ret;
ret = dm_i2c_read(chip, 0, &val, 1);
if (!ret) {
env_set("vdd_cpu", regulator_details[i].regulator_compat);
break;
}
- }
- return 0;
+}
- int rgxx3_read_board_id(void) { u32 adc_info;
@@ -485,6 +551,16 @@ int rk_board_late_init(void) printf("Failed to detect panel type\n"); }
- /*
* Skip vdd_cpu regulator detection if not needed. Warn but
* don't fail for errors in auto-detection of regulator.
*/
- if (rg3xx_model_details[gd->board_type].detect_regulator) {
ret = rgxx3_detect_regulator();
if (ret)
printf("Unable to detect vdd_cpu regulator\n");
- }
- end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
@@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob) return 0; }
+int rgxx3_regulator_fixup(void *blob) +{
- const struct powkiddy_regulators *vdd_cpu = NULL;
- int node, ret, i;
- char path[] = "/i2c@fdd40000/regulator@00";
- char name[] = "regulator@00";
- char *env;
- env = env_get("vdd_cpu");
- if (!env) {
printf("Can't get vdd_cpu env\n");
return -EINVAL;
- }
- /*
* Find the device we have in our tree, which may or may not
* be present.
*/
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
sprintf(path, "/i2c@fdd40000/regulator@%02x",
regulator_details[i].addr);
node = fdt_path_offset(blob, path);
if (node > 0)
break;
printf("Unable to find vdd_cpu\n");
return -ENODEV;
- }
- node = fdt_path_offset(blob, path);
- if (!(node > 0)) {
printf("Can't find the vdd_cpu node\n");
return -ENODEV;
- }
- ret = fdt_node_check_compatible(blob, node, env);
- if (ret < 0)
return -ENODEV;
- /* vdd_cpu regulators match, return 0. */
- if (!ret)
return 0;
- /* Regulators don't match, search by first compatible value. */
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
if (!strcmp(env, regulator_details[i].regulator_compat)) {
vdd_cpu = ®ulator_details[i];
break;
}
- }
- if (!vdd_cpu) {
printf("Unable to identify vdd_cpu by compat string\n");
return -ENODEV;
- }
- /* Set the compatible and reg with the auto-detected values */
- fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat);
- fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr);
- sprintf(name, "regulator@%02x", vdd_cpu->addr);
- fdt_set_name(blob, node, name);
I'm not sure this is viable in the long run... This relies on the properties for the two devices to be identical. Are we sure it is absolutely the case?
An option could be to have a DTSO for the "new" regulator that removes the old regulator entirely and we would simply be applying that DTSO at runtime before the kernel starts. I assume you probably want this for SPL or U-Boot proper as well so applying the DTSO there would be required as well, not sure how to do this though.
Can you please tell the HW vendor to have some way of detecting some variants of the HW instead of letting us just guess stuff? How do they even support this in their downstream kernel/bootloader???? Have they already ran out of channels on SARADC?
Cheers, Quentin

On Mon, Sep 23, 2024 at 01:21:01PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Some of the Powkiddy devices switched to using a different vendor for the vdd_cpu regulator. Unfortunately the device does not have a new revision to denote this, so users have no way of knowing in advance.
Add code to detect if a device is present at addresses 0x1c or 0x40 on the i2c0 bus and update the devicetree if needed.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 224019f9ba3..c1d1826fd14 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -43,6 +43,7 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel;
- const bool detect_regulator; const bool uart_con; };
@@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353P] = {.detect_regulator = 0,
@@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353V] = {.detect_regulator = 0,
@@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG503] = {.detect_regulator = 0,
@@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0,
.uart_con = 1, }, [RGB30] = {.detect_regulator = 0,
@@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0,
.uart_con = 0, }, [RK2023] = {.detect_regulator = 1,
@@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0,
.uart_con = 0, }, [RGARCD] = {.detect_regulator = 1,
@@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0,
.uart_con = 1, }, [RGB10MAX3] = {.detect_regulator = 0,
@@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0,
.uart_con = 0, }, /* Devices with duplicate ADC value */.detect_regulator = 1,
@@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353VS] = {.detect_regulator = 0,
@@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1,
.uart_con = 1, }, [RGARCS] = {.detect_regulator = 0,
@@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0,
.uart_con = 1, }, };.detect_regulator = 0,
@@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = { }, }; +struct powkiddy_regulators {
- const u8 addr;
- const char *regulator_compat;
+};
+static const struct powkiddy_regulators regulator_details[] = {
- {
.addr = 0x1c,
.regulator_compat = "tcs,tcs4525",
- },
- {
.addr = 0x40,
.regulator_compat = "fcs,fan53555",
- },
+};
- /*
- Start LED very early so user knows device is on. Set color
- to red.
@@ -361,6 +389,44 @@ int rgxx3_detect_display(void) return 0; } +/*
- Some of the Powkiddy devices switched the CPU regulator, but users
- are not able to determine this by looking at their hardware.
- Attempt to auto-detect this situation and fixup the device-tree.
- */
+int rgxx3_detect_regulator(void) +{
- struct udevice *bus;
- struct udevice *chip;
- u8 val;
- int ret;
- /* Get the correct i2c bus (i2c0). */
- ret = uclass_get_device_by_name(UCLASS_I2C,
"i2c@fdd40000", &bus);
- if (ret)
return ret;
- /*
* Check for all vdd_cpu regulators and read an arbitrary
* register to confirm it's present.
*/
- for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) {
ret = i2c_get_chip(bus, regulator_details[i].addr,
1, &chip);
if (ret)
return ret;
ret = dm_i2c_read(chip, 0, &val, 1);
if (!ret) {
env_set("vdd_cpu", regulator_details[i].regulator_compat);
break;
}
- }
- return 0;
+}
- int rgxx3_read_board_id(void) { u32 adc_info;
@@ -485,6 +551,16 @@ int rk_board_late_init(void) printf("Failed to detect panel type\n"); }
- /*
* Skip vdd_cpu regulator detection if not needed. Warn but
* don't fail for errors in auto-detection of regulator.
*/
- if (rg3xx_model_details[gd->board_type].detect_regulator) {
ret = rgxx3_detect_regulator();
if (ret)
printf("Unable to detect vdd_cpu regulator\n");
- }
- end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
@@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob) return 0; } +int rgxx3_regulator_fixup(void *blob) +{
- const struct powkiddy_regulators *vdd_cpu = NULL;
- int node, ret, i;
- char path[] = "/i2c@fdd40000/regulator@00";
- char name[] = "regulator@00";
- char *env;
- env = env_get("vdd_cpu");
- if (!env) {
printf("Can't get vdd_cpu env\n");
return -EINVAL;
- }
- /*
* Find the device we have in our tree, which may or may not
* be present.
*/
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
sprintf(path, "/i2c@fdd40000/regulator@%02x",
regulator_details[i].addr);
node = fdt_path_offset(blob, path);
if (node > 0)
break;
printf("Unable to find vdd_cpu\n");
return -ENODEV;
- }
- node = fdt_path_offset(blob, path);
- if (!(node > 0)) {
printf("Can't find the vdd_cpu node\n");
return -ENODEV;
- }
- ret = fdt_node_check_compatible(blob, node, env);
- if (ret < 0)
return -ENODEV;
- /* vdd_cpu regulators match, return 0. */
- if (!ret)
return 0;
- /* Regulators don't match, search by first compatible value. */
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
if (!strcmp(env, regulator_details[i].regulator_compat)) {
vdd_cpu = ®ulator_details[i];
break;
}
- }
- if (!vdd_cpu) {
printf("Unable to identify vdd_cpu by compat string\n");
return -ENODEV;
- }
- /* Set the compatible and reg with the auto-detected values */
- fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat);
- fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr);
- sprintf(name, "regulator@%02x", vdd_cpu->addr);
- fdt_set_name(blob, node, name);
I'm not sure this is viable in the long run... This relies on the properties for the two devices to be identical. Are we sure it is absolutely the case?
Yes, for the moment at least the devices are entirely identical except for the CPU regulator. Unfortunately the identical ADC IDs is why I cannot simply have an additional entry for a new hardware revision.
An option could be to have a DTSO for the "new" regulator that removes the old regulator entirely and we would simply be applying that DTSO at runtime before the kernel starts. I assume you probably want this for SPL or U-Boot proper as well so applying the DTSO there would be required as well, not sure how to do this though.
In truth I really don't need the regulator this way for U-Boot at all. Near as I can tell it defaults to being on and the right voltage. I can do a DTSO, I just figured a simple fixup would be easier (plus we're already doing a fixup for a similar situation with panel revisions).
Can you please tell the HW vendor to have some way of detecting some variants of the HW instead of letting us just guess stuff? How do they even support this in their downstream kernel/bootloader???? Have they already ran out of channels on SARADC?
I have told both Powkiddy and Anbernic this. Whether they will listen is a different matter. They haven't run out of channels on the SARADC (except for the Powkiddy X55, which is its own board and has no detection logic). In theory if we allow for a ~10% variance in the resistor values for a resistor on ADC channel 1, and the range is between 0 and 1024 (20 and 1004), that gives us about 49 total devices we could "detect" at one time, assuming no one collides.
I have no idea how Powkiddy fixes up the CPU regulator, but I know Anbernic does panel fixup with some hacky code in U-Boot and Linux proper that detects for the panel like I do. In their case it's built into the probe function of Rockchip's own "display-simple-dsi" driver. In our case we detect the panel ID in U-Boot and then fixup the device tree for Linux with the correct compatible.
Cheers, Quentin
Thank you, Chris

Hi Chris,
On 9/23/24 7:36 PM, Chris Morgan wrote:
On Mon, Sep 23, 2024 at 01:21:01PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Some of the Powkiddy devices switched to using a different vendor for the vdd_cpu regulator. Unfortunately the device does not have a new revision to denote this, so users have no way of knowing in advance.
Add code to detect if a device is present at addresses 0x1c or 0x40 on the i2c0 bus and update the devicetree if needed.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 224019f9ba3..c1d1826fd14 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -43,6 +43,7 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel;
- const bool detect_regulator; const bool uart_con; };
@@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
}, [RG353P] = {.detect_regulator = 0, .uart_con = 1,
@@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
}, [RG353V] = {.detect_regulator = 0, .uart_con = 1,
@@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1,
}, [RG503] = {.detect_regulator = 0, .uart_con = 1,
@@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0,
}, [RGB30] = {.detect_regulator = 0, .uart_con = 1,
@@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0,
}, [RK2023] = {.detect_regulator = 1, .uart_con = 0,
@@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0,
}, [RGARCD] = {.detect_regulator = 1, .uart_con = 0,
@@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0,
}, [RGB10MAX3] = {.detect_regulator = 0, .uart_con = 1,
@@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0,
}, /* Devices with duplicate ADC value */.detect_regulator = 1, .uart_con = 0,
@@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1,
}, [RG353VS] = {.detect_regulator = 0, .uart_con = 1,
@@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1,
}, [RGARCS] = {.detect_regulator = 0, .uart_con = 1,
@@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0,
}, };.detect_regulator = 0, .uart_con = 1,
@@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = { }, }; +struct powkiddy_regulators {
- const u8 addr;
- const char *regulator_compat;
+};
+static const struct powkiddy_regulators regulator_details[] = {
- {
.addr = 0x1c,
.regulator_compat = "tcs,tcs4525",
- },
- {
.addr = 0x40,
.regulator_compat = "fcs,fan53555",
- },
+};
- /*
- Start LED very early so user knows device is on. Set color
- to red.
@@ -361,6 +389,44 @@ int rgxx3_detect_display(void) return 0; } +/*
- Some of the Powkiddy devices switched the CPU regulator, but users
- are not able to determine this by looking at their hardware.
- Attempt to auto-detect this situation and fixup the device-tree.
- */
+int rgxx3_detect_regulator(void) +{
- struct udevice *bus;
- struct udevice *chip;
- u8 val;
- int ret;
- /* Get the correct i2c bus (i2c0). */
- ret = uclass_get_device_by_name(UCLASS_I2C,
"i2c@fdd40000", &bus);
- if (ret)
return ret;
- /*
* Check for all vdd_cpu regulators and read an arbitrary
* register to confirm it's present.
*/
- for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) {
ret = i2c_get_chip(bus, regulator_details[i].addr,
1, &chip);
if (ret)
return ret;
ret = dm_i2c_read(chip, 0, &val, 1);
if (!ret) {
env_set("vdd_cpu", regulator_details[i].regulator_compat);
break;
}
- }
- return 0;
+}
- int rgxx3_read_board_id(void) { u32 adc_info;
@@ -485,6 +551,16 @@ int rk_board_late_init(void) printf("Failed to detect panel type\n"); }
- /*
* Skip vdd_cpu regulator detection if not needed. Warn but
* don't fail for errors in auto-detection of regulator.
*/
- if (rg3xx_model_details[gd->board_type].detect_regulator) {
ret = rgxx3_detect_regulator();
if (ret)
printf("Unable to detect vdd_cpu regulator\n");
- }
- end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
@@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob) return 0; } +int rgxx3_regulator_fixup(void *blob) +{
- const struct powkiddy_regulators *vdd_cpu = NULL;
- int node, ret, i;
- char path[] = "/i2c@fdd40000/regulator@00";
- char name[] = "regulator@00";
- char *env;
- env = env_get("vdd_cpu");
- if (!env) {
printf("Can't get vdd_cpu env\n");
return -EINVAL;
- }
- /*
* Find the device we have in our tree, which may or may not
* be present.
*/
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
sprintf(path, "/i2c@fdd40000/regulator@%02x",
regulator_details[i].addr);
node = fdt_path_offset(blob, path);
if (node > 0)
break;
printf("Unable to find vdd_cpu\n");
return -ENODEV;
- }
- node = fdt_path_offset(blob, path);
- if (!(node > 0)) {
printf("Can't find the vdd_cpu node\n");
return -ENODEV;
- }
- ret = fdt_node_check_compatible(blob, node, env);
- if (ret < 0)
return -ENODEV;
- /* vdd_cpu regulators match, return 0. */
- if (!ret)
return 0;
- /* Regulators don't match, search by first compatible value. */
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
if (!strcmp(env, regulator_details[i].regulator_compat)) {
vdd_cpu = ®ulator_details[i];
break;
}
- }
- if (!vdd_cpu) {
printf("Unable to identify vdd_cpu by compat string\n");
return -ENODEV;
- }
- /* Set the compatible and reg with the auto-detected values */
- fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat);
- fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr);
- sprintf(name, "regulator@%02x", vdd_cpu->addr);
- fdt_set_name(blob, node, name);
I'm not sure this is viable in the long run... This relies on the properties for the two devices to be identical. Are we sure it is absolutely the case?
Yes, for the moment at least the devices are entirely identical except for the CPU regulator. Unfortunately the identical ADC IDs is why I cannot simply have an additional entry for a new hardware revision.
An option could be to have a DTSO for the "new" regulator that removes the old regulator entirely and we would simply be applying that DTSO at runtime before the kernel starts. I assume you probably want this for SPL or U-Boot proper as well so applying the DTSO there would be required as well, not sure how to do this though.
In truth I really don't need the regulator this way for U-Boot at all. Near as I can tell it defaults to being on and the right voltage. I can do a DTSO, I just figured a simple fixup would be easier (plus we're already doing a fixup for a similar situation with panel revisions).
The benefit of using DTSO is that 1) it drastically lower the complexity of the C code (or I would very much hope so). 2) you can have this upstream in the kernel and it being very explicit something fishy is going on with the HW so people are aware of those things (which also means if someone has the idea of supporting another bootloader, e.g. Barebox, LittleKernel, ..., they would know about the different variants to support and similar "hacks" to implement).
You're the maintainer of those boards and this doesn't impact any other product, so I would say this is up to you :)
Can you please tell the HW vendor to have some way of detecting some variants of the HW instead of letting us just guess stuff? How do they even support this in their downstream kernel/bootloader???? Have they already ran out of channels on SARADC?
I have told both Powkiddy and Anbernic this. Whether they will listen is a different matter. They haven't run out of channels on the SARADC (except for the Powkiddy X55, which is its own board and has no detection logic). In theory if we allow for a ~10% variance in the resistor values for a resistor on ADC channel 1, and the range is between 0 and 1024 (20 and 1004), that gives us about 49 total devices we could "detect" at one time, assuming no one collides.
The channel 1 is commonly used for the recovery button IIRC on recent Rockchip SoCs-based products, so another channel probably makes more sense.
I assume one of the possible issue is that they didn't foresee such a change would be required and forgot about those ADC channels AND the new regulator is pin-to-pin compatible with the same footprint, meaning they didn't need to redo a new PCB. That's the only "excuse" i can give them :)
I have no idea how Powkiddy fixes up the CPU regulator, but I know Anbernic does panel fixup with some hacky code in U-Boot and Linux proper that detects for the panel like I do. In their case it's built into the probe function of Rockchip's own "display-simple-dsi" driver. In our case we detect the panel ID in U-Boot and then fixup the device tree for Linux with the correct compatible.
I definitely never had to do that myself. Never! I don't know what you're talking about ;) Oh look, a squirrel.
Cheers, Quentin

On Tue, Sep 24, 2024 at 12:24:16PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/23/24 7:36 PM, Chris Morgan wrote:
On Mon, Sep 23, 2024 at 01:21:01PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Some of the Powkiddy devices switched to using a different vendor for the vdd_cpu regulator. Unfortunately the device does not have a new revision to denote this, so users have no way of knowing in advance.
Add code to detect if a device is present at addresses 0x1c or 0x40 on the i2c0 bus and update the devicetree if needed.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 224019f9ba3..c1d1826fd14 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -43,6 +43,7 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel;
- const bool detect_regulator; const bool uart_con; };
@@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
}, [RG353P] = {.detect_regulator = 0, .uart_con = 1,
@@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
}, [RG353V] = {.detect_regulator = 0, .uart_con = 1,
@@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1,
}, [RG503] = {.detect_regulator = 0, .uart_con = 1,
@@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0,
}, [RGB30] = {.detect_regulator = 0, .uart_con = 1,
@@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0,
}, [RK2023] = {.detect_regulator = 1, .uart_con = 0,
@@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0,
}, [RGARCD] = {.detect_regulator = 1, .uart_con = 0,
@@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0,
}, [RGB10MAX3] = {.detect_regulator = 0, .uart_con = 1,
@@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0,
}, /* Devices with duplicate ADC value */.detect_regulator = 1, .uart_con = 0,
@@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1,
}, [RG353VS] = {.detect_regulator = 0, .uart_con = 1,
@@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1,
}, [RGARCS] = {.detect_regulator = 0, .uart_con = 1,
@@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0,
}, };.detect_regulator = 0, .uart_con = 1,
@@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = { }, }; +struct powkiddy_regulators {
- const u8 addr;
- const char *regulator_compat;
+};
+static const struct powkiddy_regulators regulator_details[] = {
- {
.addr = 0x1c,
.regulator_compat = "tcs,tcs4525",
- },
- {
.addr = 0x40,
.regulator_compat = "fcs,fan53555",
- },
+};
- /*
- Start LED very early so user knows device is on. Set color
- to red.
@@ -361,6 +389,44 @@ int rgxx3_detect_display(void) return 0; } +/*
- Some of the Powkiddy devices switched the CPU regulator, but users
- are not able to determine this by looking at their hardware.
- Attempt to auto-detect this situation and fixup the device-tree.
- */
+int rgxx3_detect_regulator(void) +{
- struct udevice *bus;
- struct udevice *chip;
- u8 val;
- int ret;
- /* Get the correct i2c bus (i2c0). */
- ret = uclass_get_device_by_name(UCLASS_I2C,
"i2c@fdd40000", &bus);
- if (ret)
return ret;
- /*
* Check for all vdd_cpu regulators and read an arbitrary
* register to confirm it's present.
*/
- for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) {
ret = i2c_get_chip(bus, regulator_details[i].addr,
1, &chip);
if (ret)
return ret;
ret = dm_i2c_read(chip, 0, &val, 1);
if (!ret) {
env_set("vdd_cpu", regulator_details[i].regulator_compat);
break;
}
- }
- return 0;
+}
- int rgxx3_read_board_id(void) { u32 adc_info;
@@ -485,6 +551,16 @@ int rk_board_late_init(void) printf("Failed to detect panel type\n"); }
- /*
* Skip vdd_cpu regulator detection if not needed. Warn but
* don't fail for errors in auto-detection of regulator.
*/
- if (rg3xx_model_details[gd->board_type].detect_regulator) {
ret = rgxx3_detect_regulator();
if (ret)
printf("Unable to detect vdd_cpu regulator\n");
- }
- end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
@@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob) return 0; } +int rgxx3_regulator_fixup(void *blob) +{
- const struct powkiddy_regulators *vdd_cpu = NULL;
- int node, ret, i;
- char path[] = "/i2c@fdd40000/regulator@00";
- char name[] = "regulator@00";
- char *env;
- env = env_get("vdd_cpu");
- if (!env) {
printf("Can't get vdd_cpu env\n");
return -EINVAL;
- }
- /*
* Find the device we have in our tree, which may or may not
* be present.
*/
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
sprintf(path, "/i2c@fdd40000/regulator@%02x",
regulator_details[i].addr);
node = fdt_path_offset(blob, path);
if (node > 0)
break;
printf("Unable to find vdd_cpu\n");
return -ENODEV;
- }
- node = fdt_path_offset(blob, path);
- if (!(node > 0)) {
printf("Can't find the vdd_cpu node\n");
return -ENODEV;
- }
- ret = fdt_node_check_compatible(blob, node, env);
- if (ret < 0)
return -ENODEV;
- /* vdd_cpu regulators match, return 0. */
- if (!ret)
return 0;
- /* Regulators don't match, search by first compatible value. */
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
if (!strcmp(env, regulator_details[i].regulator_compat)) {
vdd_cpu = ®ulator_details[i];
break;
}
- }
- if (!vdd_cpu) {
printf("Unable to identify vdd_cpu by compat string\n");
return -ENODEV;
- }
- /* Set the compatible and reg with the auto-detected values */
- fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat);
- fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr);
- sprintf(name, "regulator@%02x", vdd_cpu->addr);
- fdt_set_name(blob, node, name);
I'm not sure this is viable in the long run... This relies on the properties for the two devices to be identical. Are we sure it is absolutely the case?
Yes, for the moment at least the devices are entirely identical except for the CPU regulator. Unfortunately the identical ADC IDs is why I cannot simply have an additional entry for a new hardware revision.
An option could be to have a DTSO for the "new" regulator that removes the old regulator entirely and we would simply be applying that DTSO at runtime before the kernel starts. I assume you probably want this for SPL or U-Boot proper as well so applying the DTSO there would be required as well, not sure how to do this though.
In truth I really don't need the regulator this way for U-Boot at all. Near as I can tell it defaults to being on and the right voltage. I can do a DTSO, I just figured a simple fixup would be easier (plus we're already doing a fixup for a similar situation with panel revisions).
The benefit of using DTSO is that 1) it drastically lower the complexity of the C code (or I would very much hope so). 2) you can have this upstream in the kernel and it being very explicit something fishy is going on with the HW so people are aware of those things (which also means if someone has the idea of supporting another bootloader, e.g. Barebox, LittleKernel, ..., they would know about the different variants to support and similar "hacks" to implement).
You're the maintainer of those boards and this doesn't impact any other product, so I would say this is up to you :)
The difference in code between "detect problem" and "detect problem and fix" is about 3 lines, so I might as well just fix the problem when we find it.
Long term if the device tree lets us specify multiple things [1] and we can somehow probe for which one is present that would be ideal, but for now this works so I say "ship it".
[1] This is kind of interesting and could solve this and other problems I face. I'm definitely keeping my eye on it: https://lore.kernel.org/linux-devicetree/20240911072751.365361-1-wenst@chrom...
Thank you, Chris
Can you please tell the HW vendor to have some way of detecting some variants of the HW instead of letting us just guess stuff? How do they even support this in their downstream kernel/bootloader???? Have they already ran out of channels on SARADC?
I have told both Powkiddy and Anbernic this. Whether they will listen is a different matter. They haven't run out of channels on the SARADC (except for the Powkiddy X55, which is its own board and has no detection logic). In theory if we allow for a ~10% variance in the resistor values for a resistor on ADC channel 1, and the range is between 0 and 1024 (20 and 1004), that gives us about 49 total devices we could "detect" at one time, assuming no one collides.
The channel 1 is commonly used for the recovery button IIRC on recent Rockchip SoCs-based products, so another channel probably makes more sense.
I assume one of the possible issue is that they didn't foresee such a change would be required and forgot about those ADC channels AND the new regulator is pin-to-pin compatible with the same footprint, meaning they didn't need to redo a new PCB. That's the only "excuse" i can give them :)
I have no idea how Powkiddy fixes up the CPU regulator, but I know Anbernic does panel fixup with some hacky code in U-Boot and Linux proper that detects for the panel like I do. In their case it's built into the probe function of Rockchip's own "display-simple-dsi" driver. In our case we detect the panel ID in U-Boot and then fixup the device tree for Linux with the correct compatible.
I definitely never had to do that myself. Never! I don't know what you're talking about ;) Oh look, a squirrel.
Cheers, Quentin

On 2024/9/19 22:00, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Some of the Powkiddy devices switched to using a different vendor for the vdd_cpu regulator. Unfortunately the device does not have a new revision to denote this, so users have no way of knowing in advance.
Add code to detect if a device is present at addresses 0x1c or 0x40 on the i2c0 bus and update the devicetree if needed.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c index 224019f9ba3..c1d1826fd14 100644 --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c @@ -43,6 +43,7 @@ struct rg3xx_model { const char *board_name; const char *fdtfile; const bool detect_panel;
- const bool detect_regulator; const bool uart_con; };
@@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { /* Device is identical to RG353P. */ .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353P] = {.detect_regulator = 0,
@@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353P", .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353V] = {.detect_regulator = 0,
@@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353V", .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG503] = {.detect_regulator = 0,
@@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG503", .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb", .detect_panel = 0,
.uart_con = 1, }, [RGB30] = {.detect_regulator = 0,
@@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB30", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb", .detect_panel = 0,
.uart_con = 0, }, [RK2023] = {.detect_regulator = 1,
@@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RK2023", .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb", .detect_panel = 0,
.uart_con = 0, }, [RGARCD] = {.detect_regulator = 1,
@@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-D", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb", .detect_panel = 0,
.uart_con = 1, }, [RGB10MAX3] = {.detect_regulator = 0,
@@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Powkiddy RGB10MAX3", .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb", .detect_panel = 0,
.uart_con = 0, }, /* Devices with duplicate ADC value */.detect_regulator = 1,
@@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353PS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb", .detect_panel = 1,
.uart_con = 1, }, [RG353VS] = {.detect_regulator = 0,
@@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG353VS", .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb", .detect_panel = 1,
.uart_con = 1, }, [RGARCS] = {.detect_regulator = 0,
@@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = { .board_name = "Anbernic RG ARC-S", .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb", .detect_panel = 0,
.uart_con = 1, }, };.detect_regulator = 0,
@@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = { }, };
+struct powkiddy_regulators {
- const u8 addr;
- const char *regulator_compat;
+};
+static const struct powkiddy_regulators regulator_details[] = {
- {
.addr = 0x1c,
.regulator_compat = "tcs,tcs4525",
- },
- {
.addr = 0x40,
.regulator_compat = "fcs,fan53555",
- },
+};
- /*
- Start LED very early so user knows device is on. Set color
- to red.
@@ -361,6 +389,44 @@ int rgxx3_detect_display(void) return 0; }
+/*
- Some of the Powkiddy devices switched the CPU regulator, but users
- are not able to determine this by looking at their hardware.
- Attempt to auto-detect this situation and fixup the device-tree.
- */
+int rgxx3_detect_regulator(void) +{
- struct udevice *bus;
- struct udevice *chip;
- u8 val;
- int ret;
- /* Get the correct i2c bus (i2c0). */
- ret = uclass_get_device_by_name(UCLASS_I2C,
"i2c@fdd40000", &bus);
- if (ret)
return ret;
- /*
* Check for all vdd_cpu regulators and read an arbitrary
* register to confirm it's present.
*/
- for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) {
ret = i2c_get_chip(bus, regulator_details[i].addr,
1, &chip);
if (ret)
return ret;
ret = dm_i2c_read(chip, 0, &val, 1);
if (!ret) {
env_set("vdd_cpu", regulator_details[i].regulator_compat);
break;
}
- }
- return 0;
+}
- int rgxx3_read_board_id(void) { u32 adc_info;
@@ -485,6 +551,16 @@ int rk_board_late_init(void) printf("Failed to detect panel type\n"); }
- /*
* Skip vdd_cpu regulator detection if not needed. Warn but
* don't fail for errors in auto-detection of regulator.
*/
- if (rg3xx_model_details[gd->board_type].detect_regulator) {
ret = rgxx3_detect_regulator();
if (ret)
printf("Unable to detect vdd_cpu regulator\n");
- }
- end: /* Turn off red LED and turn on orange LED. */ writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
@@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob) return 0; }
+int rgxx3_regulator_fixup(void *blob) +{
- const struct powkiddy_regulators *vdd_cpu = NULL;
- int node, ret, i;
- char path[] = "/i2c@fdd40000/regulator@00";
- char name[] = "regulator@00";
- char *env;
- env = env_get("vdd_cpu");
- if (!env) {
printf("Can't get vdd_cpu env\n");
return -EINVAL;
- }
- /*
* Find the device we have in our tree, which may or may not
* be present.
*/
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
sprintf(path, "/i2c@fdd40000/regulator@%02x",
regulator_details[i].addr);
node = fdt_path_offset(blob, path);
if (node > 0)
break;
printf("Unable to find vdd_cpu\n");
return -ENODEV;
- }
- node = fdt_path_offset(blob, path);
- if (!(node > 0)) {
printf("Can't find the vdd_cpu node\n");
return -ENODEV;
- }
- ret = fdt_node_check_compatible(blob, node, env);
- if (ret < 0)
return -ENODEV;
- /* vdd_cpu regulators match, return 0. */
- if (!ret)
return 0;
- /* Regulators don't match, search by first compatible value. */
- for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
if (!strcmp(env, regulator_details[i].regulator_compat)) {
vdd_cpu = ®ulator_details[i];
break;
}
- }
- if (!vdd_cpu) {
printf("Unable to identify vdd_cpu by compat string\n");
return -ENODEV;
- }
- /* Set the compatible and reg with the auto-detected values */
- fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat);
- fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr);
- sprintf(name, "regulator@%02x", vdd_cpu->addr);
- fdt_set_name(blob, node, name);
- return 0;
+}
- int ft_board_setup(void *blob, struct bd_info *bd) { int ret;
@@ -562,6 +703,12 @@ int ft_board_setup(void *blob, struct bd_info *bd) printf("Unable to update panel compat\n"); }
- if (rg3xx_model_details[gd->board_type].detect_regulator) {
ret = rgxx3_regulator_fixup(blob);
if (ret)
printf("Unable to update vdd_cpu compat\n");
- }
- return 0; }

From: Chris Morgan macromorgan@hotmail.com
Remove config options for ARM SCMI. It is not required to boot the board and when using the most recent mainline A-TF it actually causes the device to freeze during boot due to missing SCMI support.
Signed-off-by: Chris Morgan macromorgan@hotmail.com --- configs/anbernic-rgxx3-rk3566_defconfig | 2 -- 1 file changed, 2 deletions(-)
diff --git a/configs/anbernic-rgxx3-rk3566_defconfig b/configs/anbernic-rgxx3-rk3566_defconfig index f5e5470df8c..49d3766613e 100644 --- a/configs/anbernic-rgxx3-rk3566_defconfig +++ b/configs/anbernic-rgxx3-rk3566_defconfig @@ -49,8 +49,6 @@ CONFIG_SPL_REGMAP=y CONFIG_SPL_SYSCON=y CONFIG_SPL_ADC=y CONFIG_SPL_CLK=y -CONFIG_ARM_SMCCC_FEATURES=y -CONFIG_SCMI_FIRMWARE=y CONFIG_ROCKCHIP_GPIO=y CONFIG_SYS_I2C_ROCKCHIP=y CONFIG_MISC=y

On 2024/9/19 22:00, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Remove config options for ARM SCMI. It is not required to boot the board and when using the most recent mainline A-TF it actually causes the device to freeze during boot due to missing SCMI support.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
configs/anbernic-rgxx3-rk3566_defconfig | 2 -- 1 file changed, 2 deletions(-)
diff --git a/configs/anbernic-rgxx3-rk3566_defconfig b/configs/anbernic-rgxx3-rk3566_defconfig index f5e5470df8c..49d3766613e 100644 --- a/configs/anbernic-rgxx3-rk3566_defconfig +++ b/configs/anbernic-rgxx3-rk3566_defconfig @@ -49,8 +49,6 @@ CONFIG_SPL_REGMAP=y CONFIG_SPL_SYSCON=y CONFIG_SPL_ADC=y CONFIG_SPL_CLK=y -CONFIG_ARM_SMCCC_FEATURES=y -CONFIG_SCMI_FIRMWARE=y CONFIG_ROCKCHIP_GPIO=y CONFIG_SYS_I2C_ROCKCHIP=y CONFIG_MISC=y

From: Chris Morgan macromorgan@hotmail.com
Enable the PD_VO power domain before driver access on the rk3568 SoC.
Signed-off-by: Chris Morgan macromorgan@hotmail.com --- arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index 1b3e40074e3..d5742b68d68 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -26,6 +26,8 @@ #define PMU_BASE_ADDR 0xfdd90000 #define PMU_NOC_AUTO_CON0 (0x70) #define PMU_NOC_AUTO_CON1 (0x74) +#define PMU_PWR_GATE_SFTCON (0xa0) +#define PMU_PD_VO_DWN_ENA BIT(7) #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) @@ -130,6 +132,10 @@ int arch_cpu_init(void) writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3); + + /* Enable VO power domain for display */ + writel((PMU_PD_VO_DWN_ENA << 16), + PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON); #endif return 0; }

Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Enable the PD_VO power domain before driver access on the rk3568 SoC.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index 1b3e40074e3..d5742b68d68 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -26,6 +26,8 @@ #define PMU_BASE_ADDR 0xfdd90000 #define PMU_NOC_AUTO_CON0 (0x70) #define PMU_NOC_AUTO_CON1 (0x74) +#define PMU_PWR_GATE_SFTCON (0xa0) +#define PMU_PD_VO_DWN_ENA BIT(7) #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) @@ -130,6 +132,10 @@ int arch_cpu_init(void) writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3);
- /* Enable VO power domain for display */
- writel((PMU_PD_VO_DWN_ENA << 16),
PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);
Maybe we want to guard this with a symbol for video stuff in U-Boot? I assume Linux is capable of handling this just fine without U-Boot's intervention?
At the very least have #if CONFIG_IS_ENABLED(VIDEO) ?
What do you think?
Cheers, Quentin

On Mon, Sep 23, 2024 at 01:24:34PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Enable the PD_VO power domain before driver access on the rk3568 SoC.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index 1b3e40074e3..d5742b68d68 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -26,6 +26,8 @@ #define PMU_BASE_ADDR 0xfdd90000 #define PMU_NOC_AUTO_CON0 (0x70) #define PMU_NOC_AUTO_CON1 (0x74) +#define PMU_PWR_GATE_SFTCON (0xa0) +#define PMU_PD_VO_DWN_ENA BIT(7) #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) @@ -130,6 +132,10 @@ int arch_cpu_init(void) writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3);
- /* Enable VO power domain for display */
- writel((PMU_PD_VO_DWN_ENA << 16),
PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);
Maybe we want to guard this with a symbol for video stuff in U-Boot? I assume Linux is capable of handling this just fine without U-Boot's intervention?
At the very least have #if CONFIG_IS_ENABLED(VIDEO) ?
What do you think?
I'm game if you are. To me this just brings back the default behavior of the binary A-TF from Rockchip where the PD_VO was enabled, but if we want to guard this I'm okay.
Cheers, Quentin
Thank you, Chris

Hi Chris,
On 9/23/24 7:38 PM, Chris Morgan wrote:
On Mon, Sep 23, 2024 at 01:24:34PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Enable the PD_VO power domain before driver access on the rk3568 SoC.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index 1b3e40074e3..d5742b68d68 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -26,6 +26,8 @@ #define PMU_BASE_ADDR 0xfdd90000 #define PMU_NOC_AUTO_CON0 (0x70) #define PMU_NOC_AUTO_CON1 (0x74) +#define PMU_PWR_GATE_SFTCON (0xa0) +#define PMU_PD_VO_DWN_ENA BIT(7) #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) @@ -130,6 +132,10 @@ int arch_cpu_init(void) writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3);
- /* Enable VO power domain for display */
- writel((PMU_PD_VO_DWN_ENA << 16),
PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);
Maybe we want to guard this with a symbol for video stuff in U-Boot? I assume Linux is capable of handling this just fine without U-Boot's intervention?
At the very least have #if CONFIG_IS_ENABLED(VIDEO) ?
What do you think?
I'm game if you are. To me this just brings back the default behavior of the binary A-TF from Rockchip where the PD_VO was enabled, but if we want to guard this I'm okay.
Just to be sure I understand correctly...
Would a device without this power domain enabled by U-Boot still have working video in Linux with upstream TF-A?
Cheers, Quentin

On Tue, Sep 24, 2024 at 11:19:49AM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/23/24 7:38 PM, Chris Morgan wrote:
On Mon, Sep 23, 2024 at 01:24:34PM +0200, Quentin Schulz wrote:
Hi Chris,
On 9/19/24 4:00 PM, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Enable the PD_VO power domain before driver access on the rk3568 SoC.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index 1b3e40074e3..d5742b68d68 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -26,6 +26,8 @@ #define PMU_BASE_ADDR 0xfdd90000 #define PMU_NOC_AUTO_CON0 (0x70) #define PMU_NOC_AUTO_CON1 (0x74) +#define PMU_PWR_GATE_SFTCON (0xa0) +#define PMU_PD_VO_DWN_ENA BIT(7) #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) @@ -130,6 +132,10 @@ int arch_cpu_init(void) writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3);
- /* Enable VO power domain for display */
- writel((PMU_PD_VO_DWN_ENA << 16),
PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);
Maybe we want to guard this with a symbol for video stuff in U-Boot? I assume Linux is capable of handling this just fine without U-Boot's intervention?
At the very least have #if CONFIG_IS_ENABLED(VIDEO) ?
What do you think?
I'm game if you are. To me this just brings back the default behavior of the binary A-TF from Rockchip where the PD_VO was enabled, but if we want to guard this I'm okay.
Just to be sure I understand correctly...
Would a device without this power domain enabled by U-Boot still have working video in Linux with upstream TF-A?
Ideally yes. I assume this requires a functional power domain driver in Linux. If you lack such then video won't work without this patch, conversely if you lack such and don't use video then you'll have higher power draw with this patch.
Honestly I figure we can guard it for now and if we find upstream video issues on mainline A-TF we know this is a great place to start for troubleshooting it.
Chris
Cheers, Quentin

On 2024/9/19 22:00, Chris Morgan wrote:
From: Chris Morgan macromorgan@hotmail.com
Enable the PD_VO power domain before driver access on the rk3568 SoC.
Signed-off-by: Chris Morgan macromorgan@hotmail.com
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
arch/arm/mach-rockchip/rk3568/rk3568.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index 1b3e40074e3..d5742b68d68 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -26,6 +26,8 @@ #define PMU_BASE_ADDR 0xfdd90000 #define PMU_NOC_AUTO_CON0 (0x70) #define PMU_NOC_AUTO_CON1 (0x74) +#define PMU_PWR_GATE_SFTCON (0xa0) +#define PMU_PD_VO_DWN_ENA BIT(7) #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) @@ -130,6 +132,10 @@ int arch_cpu_init(void) writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_1); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_2); writel(0x3f3f0707, GRF_BASE + GRF_GPIO1C_DS_3);
- /* Enable VO power domain for display */
- writel((PMU_PD_VO_DWN_ENA << 16),
#endif return 0; }PMU_BASE_ADDR + PMU_PWR_GATE_SFTCON);

Hi Chris,
Update the Anbernic RGxx3 "device" to use upstream device-trees, add logic to detect a different vdd_cpu regulator, and implement a fix to allow the panel auto-detection to run when using mainline A-TF.
Note that *Linux* still cannot use mainline A-TF because of the missing SCMI clock and reset functionality, but this patch series at least ensures that this board can boot once Linux is ready.
Have you got a link to this upstream in the kernel lists? Is there patches posted? I think I have seen a similar issue when playing with mainline TF-A the other day on the Quartz64.
Peter
Chris Morgan (4): board: rockchip: Convert Anbernic RGxx3 to OF_UPSTREAM board: rockchip: Add vdd_cpu reg fixup for RGXX3 Series board: rockchip: Remove ARM SCMI Support from RGxx3 board: rockchip: Enable PD_VO before driver access
.../dts/rk3566-anbernic-rg353p-u-boot.dtsi | 34 ++ .../arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi | 52 --- arch/arm/dts/rk3566-anbernic-rgxx3.dts | 28 -- arch/arm/mach-rockchip/rk3568/rk3568.c | 6 + board/anbernic/rgxx3_rk3566/MAINTAINERS | 4 +- board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 359 +++++++++++++++--- configs/anbernic-rgxx3-rk3566_defconfig | 12 +- 7 files changed, 347 insertions(+), 148 deletions(-) create mode 100644 arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3.dts
-- 2.34.1

On Sun, Sep 22, 2024 at 02:43:11PM +0100, Peter Robinson wrote:
Hi Chris,
Update the Anbernic RGxx3 "device" to use upstream device-trees, add logic to detect a different vdd_cpu regulator, and implement a fix to allow the panel auto-detection to run when using mainline A-TF.
Note that *Linux* still cannot use mainline A-TF because of the missing SCMI clock and reset functionality, but this patch series at least ensures that this board can boot once Linux is ready.
Have you got a link to this upstream in the kernel lists? Is there patches posted? I think I have seen a similar issue when playing with mainline TF-A the other day on the Quartz64.
Peter
I don't. I just remember when I was testing A-TF previously (I've been following the RK3568 A-TF status since it was first submitted) that it would fail because of missing cpu clocking. Basically the cpu device tree node has a defined link to the SCMI clock that's provided by A-TF. Except that mainline A-TF doesn't do this, so it would get stuck in deferred probe hell. The *real* solution I think is to get SCMI clk and reset support in mainline A-TF, but unfortunately I don't know how to do that at the moment.
Chris
Chris Morgan (4): board: rockchip: Convert Anbernic RGxx3 to OF_UPSTREAM board: rockchip: Add vdd_cpu reg fixup for RGXX3 Series board: rockchip: Remove ARM SCMI Support from RGxx3 board: rockchip: Enable PD_VO before driver access
.../dts/rk3566-anbernic-rg353p-u-boot.dtsi | 34 ++ .../arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi | 52 --- arch/arm/dts/rk3566-anbernic-rgxx3.dts | 28 -- arch/arm/mach-rockchip/rk3568/rk3568.c | 6 + board/anbernic/rgxx3_rk3566/MAINTAINERS | 4 +- board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 359 +++++++++++++++--- configs/anbernic-rgxx3-rk3566_defconfig | 12 +- 7 files changed, 347 insertions(+), 148 deletions(-) create mode 100644 arch/arm/dts/rk3566-anbernic-rg353p-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi delete mode 100644 arch/arm/dts/rk3566-anbernic-rgxx3.dts
-- 2.34.1
participants (5)
-
Chris Morgan
-
Chris Morgan
-
Kever Yang
-
Peter Robinson
-
Quentin Schulz