Re: [PATCH 1/2] rockchip: rk3588: add support for UART2 M1 and M2 in SPL
Hi Quentin, On 2024/4/23 17:31, Quentin Schulz wrote: Hi Kever, On 4/23/24 03:09, Kever Yang wrote: Hi Quentin, On 2024/4/23 00:41, Quentin Schulz wrote: From: Quentin Schulz UART2 controller is the controller in the reference design for debug console. The default mux is M0 in that reference design. Until now, all boards seemed to be using UART2M0 but RK3588 Tiger for example will be using UART2M2 instead. This feature already been supported, please use CONFIG_DEBUG_UART_BASE and CONFIG_ROCKCHIP_UART_MUX_SEL_M to select your output channel. CONFIG_ROCKCHIP_UART_MUX_SEL_M is only available in Rockchip's downstream U-Boot. Sorry, my mistake, I'm searching the wrong code. I though we should already have solution for this, it should be DEBUG_UART_CHANNEL then. Thanks, - Kever git log -p -S ROCKCHIP_UART_MUX_SEL_M returns nothing upstream, neither does git grep. I used the same mechanism as for PX30. This patch will be removed though as it's not needed until we have an open-source DRAM init. Until then we have to rely on the DDR bin to setup the UART. Cheers, Quentin
Re: [PATCH 1/2] rockchip: rk3588: add support for UART2 M1 and M2 in SPL
Hi Kever, On 4/23/24 03:09, Kever Yang wrote: Hi Quentin, On 2024/4/23 00:41, Quentin Schulz wrote: From: Quentin Schulz UART2 controller is the controller in the reference design for debug console. The default mux is M0 in that reference design. Until now, all boards seemed to be using UART2M0 but RK3588 Tiger for example will be using UART2M2 instead. This feature already been supported, please use CONFIG_DEBUG_UART_BASE and CONFIG_ROCKCHIP_UART_MUX_SEL_M to select your output channel. CONFIG_ROCKCHIP_UART_MUX_SEL_M is only available in Rockchip's downstream U-Boot. git log -p -S ROCKCHIP_UART_MUX_SEL_M returns nothing upstream, neither does git grep. I used the same mechanism as for PX30. This patch will be removed though as it's not needed until we have an open-source DRAM init. Until then we have to rely on the DDR bin to setup the UART. Cheers, Quentin
Re: [PATCH 1/2] rockchip: rk3588: add support for UART2 M1 and M2 in SPL
Hi Jonas, On 4/22/24 19:41, Jonas Karlman wrote: Hi Quentin, On 2024-04-22 18:41, Quentin Schulz wrote: From: Quentin Schulz UART2 controller is the controller in the reference design for debug console. The default mux is M0 in that reference design. Until now, all boards seemed to be using UART2M0 but RK3588 Tiger for example will be using UART2M2 instead. Therefore, let's add support for UART2M1 and M2 as possible muxes for the UART2 controller used as debug console. UART2M1 support was not tested. The default value is M0 to match the one used currently by all devices and the reference design. Is this really necessary? Use of board_debug_uart_init() should typically only be needed in TPL on Rockchip platform, and with ROCKCHIP_TPL being used it should be enough to use rkbin/ddrbin_tool to change uart config and just ensure correct pinctrl is used for uart node, and that the uart node is included in SPL for correct serial console use. ddrbin_tool is a blob that Rockchip refuses to provide sources of. Running a blob on the target is one thing, requiring our users to run a blob on their build machine is another thing (though I document it in the rST). However... I don't think we have another way around because I just remembered that if you have two muxes selected for the same UART controller, RX won't work. So while we would have UART output for U-Boot if Rockchip's TPL is one mux (e.g. m0, the default) and upstream U-Boot another one, we wouldn't be able to interact with it. It'll be necessary the day we have an open-source DRAM init though (I had to do this for PX30 for example). The issue is that since ddrbin_tool is a blob, it's not possible to use it in Yocto for automatically generating the appropriate ddr bin blob based on uart controller, mux and baudrate. So that will be my cross to bear. May I suggest you try adding following to defconfig and drop this patch? # CONFIG_DEBUG_UART_BOARD_INIT is not set I would expect that should result in same/working behavior without having to add any new code. It does work, thanks for the suggestion, will send a v2. Cheers, Quentin
Re: [PATCH 1/2] rockchip: rk3588: add support for UART2 M1 and M2 in SPL
Hi Quentin,
On 2024/4/23 00:41, Quentin Schulz wrote:
From: Quentin Schulz
UART2 controller is the controller in the reference design for debug
console. The default mux is M0 in that reference design. Until now, all
boards seemed to be using UART2M0 but RK3588 Tiger for example will be
using UART2M2 instead.
This feature already been supported, please use CONFIG_DEBUG_UART_BASE and
CONFIG_ROCKCHIP_UART_MUX_SEL_M to select your output channel.
Thanks,
- Kever
Therefore, let's add support for UART2M1 and M2 as possible muxes for
the UART2 controller used as debug console. UART2M1 support was not
tested.
The default value is M0 to match the one used currently by all devices
and the reference design.
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
---
arch/arm/mach-rockchip/rk3588/Kconfig | 10 ++
arch/arm/mach-rockchip/rk3588/rk3588.c | 36 ++
2 files changed, 46 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3588/Kconfig
b/arch/arm/mach-rockchip/rk3588/Kconfig
index d7e4af31f24..cacdb0459c9 100644
--- a/arch/arm/mach-rockchip/rk3588/Kconfig
+++ b/arch/arm/mach-rockchip/rk3588/Kconfig
@@ -221,6 +221,16 @@ config ROCKCHIP_COMMON_STACK_ADDR
config TEXT_BASE
default 0x00a0
+config DEBUG_UART_CHANNEL
+ int "Mux channel to use for debug UART2"
+ depends on DEBUG_UART_BOARD_INIT
+ default 0
+ range 0 2
+ help
+ UART2 can use three different set of pins to route the output.
+ For using the UART for early debugging the route to use needs
+ to be declared (0, 1 or 2).
+
source board/edgeble/neural-compute-module-6/Kconfig
source board/friendlyelec/nanopc-t6-rk3588/Kconfig
source board/pine64/quartzpro64-rk3588/Kconfig
diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c
b/arch/arm/mach-rockchip/rk3588/rk3588.c
index eb65dafe3a2..e330ad6a697 100644
--- a/arch/arm/mach-rockchip/rk3588/rk3588.c
+++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
@@ -94,9 +94,32 @@ enum {
GPIO0B6_UART2_RX_M0 = 10,
};
+/* GPIO3B_IOMUX_SEL_L */
+enum {
+ GPIO3B1_SHIFT = 4,
+ GPIO3B1_MASK= GENMASK(7, 4),
+ GPIO3B1_UART2_TX_M2 = 10,
+
+ GPIO3B2_SHIFT = 8,
+ GPIO3B2_MASK= GENMASK(11, 8),
+ GPIO3B2_UART2_RX_M2 = 10,
+};
+
+/* GPIO4D_IOMUX_SEL_L */
+enum {
+ GPIO4D0_SHIFT = 0,
+ GPIO4D0_MASK= GENMASK(3, 0),
+ GPIO4D0_UART2_TX_M1 = 10,
+
+ GPIO4D1_SHIFT = 4,
+ GPIO4D1_MASK= GENMASK(7, 4),
+ GPIO4D1_UART2_RX_M1 = 10,
+};
+
void board_debug_uart_init(void)
{
__maybe_unused static struct rk3588_bus_ioc * const bus_ioc = (void
*)BUS_IOC_BASE;
+#if (CONFIG_DEBUG_UART_CHANNEL == 0)
static struct rk3588_pmu2_ioc * const pmu2_ioc = (void *)PMU2_IOC_BASE;
/* Refer to BUS_IOC */
@@ -110,6 +133,19 @@ void board_debug_uart_init(void)
GPIO0B6_MASK | GPIO0B5_MASK,
GPIO0B6_UART2_RX_M0 << GPIO0B6_SHIFT |
GPIO0B5_UART2_TX_M0 << GPIO0B5_SHIFT);
+#elif (CONFIG_DEBUG_UART_CHANNEL == 1)
+ /* UART2_M1 Switch iomux */
+ rk_clrsetreg(_ioc->gpio4d_iomux_sel_l,
+GPIO4D0_MASK | GPIO4D1_MASK,
+GPIO4D0_UART2_TX_M1 << GPIO4D0_UART2_TX_M1 |
+GPIO4D1_UART2_RX_M1 << GPIO4D1_SHIFT);
+#else
+ /* UART2_M2 Switch iomux */
+ rk_clrsetreg(_ioc->gpio3b_iomux_sel_l,
+GPIO3B1_MASK | GPIO3B2_MASK,
+GPIO3B1_UART2_TX_M2 << GPIO3B1_SHIFT |
+GPIO3B2_UART2_RX_M2 << GPIO3B2_SHIFT);
+#endif /* CONFIG_DEBUG_UART_CHANNEL */
}
#ifdef CONFIG_SPL_BUILD
Re: [PATCH 1/2] rockchip: rk3588: add support for UART2 M1 and M2 in SPL
Hi Quentin,
On 2024-04-22 18:41, Quentin Schulz wrote:
> From: Quentin Schulz
>
> UART2 controller is the controller in the reference design for debug
> console. The default mux is M0 in that reference design. Until now, all
> boards seemed to be using UART2M0 but RK3588 Tiger for example will be
> using UART2M2 instead.
>
> Therefore, let's add support for UART2M1 and M2 as possible muxes for
> the UART2 controller used as debug console. UART2M1 support was not
> tested.
>
> The default value is M0 to match the one used currently by all devices
> and the reference design.
Is this really necessary?
Use of board_debug_uart_init() should typically only be needed in TPL on
Rockchip platform, and with ROCKCHIP_TPL being used it should be enough
to use rkbin/ddrbin_tool to change uart config and just ensure correct
pinctrl is used for uart node, and that the uart node is included in SPL
for correct serial console use.
May I suggest you try adding following to defconfig and drop this patch?
# CONFIG_DEBUG_UART_BOARD_INIT is not set
I would expect that should result in same/working behavior without
having to add any new code.
Regards,
Jonas
>
> Cc: Quentin Schulz
> Signed-off-by: Quentin Schulz
> ---
> arch/arm/mach-rockchip/rk3588/Kconfig | 10 ++
> arch/arm/mach-rockchip/rk3588/rk3588.c | 36
> ++
> 2 files changed, 46 insertions(+)
>
> diff --git a/arch/arm/mach-rockchip/rk3588/Kconfig
> b/arch/arm/mach-rockchip/rk3588/Kconfig
> index d7e4af31f24..cacdb0459c9 100644
> --- a/arch/arm/mach-rockchip/rk3588/Kconfig
> +++ b/arch/arm/mach-rockchip/rk3588/Kconfig
> @@ -221,6 +221,16 @@ config ROCKCHIP_COMMON_STACK_ADDR
> config TEXT_BASE
> default 0x00a0
>
> +config DEBUG_UART_CHANNEL
> + int "Mux channel to use for debug UART2"
> + depends on DEBUG_UART_BOARD_INIT
> + default 0
> + range 0 2
> + help
> + UART2 can use three different set of pins to route the output.
> + For using the UART for early debugging the route to use needs
> + to be declared (0, 1 or 2).
> +
> source board/edgeble/neural-compute-module-6/Kconfig
> source board/friendlyelec/nanopc-t6-rk3588/Kconfig
> source board/pine64/quartzpro64-rk3588/Kconfig
> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c
> b/arch/arm/mach-rockchip/rk3588/rk3588.c
> index eb65dafe3a2..e330ad6a697 100644
> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
> @@ -94,9 +94,32 @@ enum {
> GPIO0B6_UART2_RX_M0 = 10,
> };
>
> +/* GPIO3B_IOMUX_SEL_L */
> +enum {
> + GPIO3B1_SHIFT = 4,
> + GPIO3B1_MASK= GENMASK(7, 4),
> + GPIO3B1_UART2_TX_M2 = 10,
> +
> + GPIO3B2_SHIFT = 8,
> + GPIO3B2_MASK= GENMASK(11, 8),
> + GPIO3B2_UART2_RX_M2 = 10,
> +};
> +
> +/* GPIO4D_IOMUX_SEL_L */
> +enum {
> + GPIO4D0_SHIFT = 0,
> + GPIO4D0_MASK= GENMASK(3, 0),
> + GPIO4D0_UART2_TX_M1 = 10,
> +
> + GPIO4D1_SHIFT = 4,
> + GPIO4D1_MASK= GENMASK(7, 4),
> + GPIO4D1_UART2_RX_M1 = 10,
> +};
> +
> void board_debug_uart_init(void)
> {
> __maybe_unused static struct rk3588_bus_ioc * const bus_ioc = (void
> *)BUS_IOC_BASE;
> +#if (CONFIG_DEBUG_UART_CHANNEL == 0)
> static struct rk3588_pmu2_ioc * const pmu2_ioc = (void *)PMU2_IOC_BASE;
>
> /* Refer to BUS_IOC */
> @@ -110,6 +133,19 @@ void board_debug_uart_init(void)
>GPIO0B6_MASK | GPIO0B5_MASK,
>GPIO0B6_UART2_RX_M0 << GPIO0B6_SHIFT |
>GPIO0B5_UART2_TX_M0 << GPIO0B5_SHIFT);
> +#elif (CONFIG_DEBUG_UART_CHANNEL == 1)
> + /* UART2_M1 Switch iomux */
> + rk_clrsetreg(_ioc->gpio4d_iomux_sel_l,
> + GPIO4D0_MASK | GPIO4D1_MASK,
> + GPIO4D0_UART2_TX_M1 << GPIO4D0_UART2_TX_M1 |
> + GPIO4D1_UART2_RX_M1 << GPIO4D1_SHIFT);
> +#else
> + /* UART2_M2 Switch iomux */
> + rk_clrsetreg(_ioc->gpio3b_iomux_sel_l,
> + GPIO3B1_MASK | GPIO3B2_MASK,
> + GPIO3B1_UART2_TX_M2 << GPIO3B1_SHIFT |
> + GPIO3B2_UART2_RX_M2 << GPIO3B2_SHIFT);
> +#endif /* CONFIG_DEBUG_UART_CHANNEL */
> }
>
> #ifdef CONFIG_SPL_BUILD
>
