Re: [PATCH] rockchip: rk3308: Drop unused rk_board_init()

Jonas Karlman
2024-11-11T09:41:55Z

Hi Kever,

On 2024-11-07 02:45, Kever Yang wrote:
> Hi Jonas,
>
> On 2024/11/3 04:45, Jonas Karlman wrote:
>> Nothing is calling the function rk_board_init() and the io-domain driver
>> can handle the functions intended purpose based on information from DT.
>
> Yes, this  should be take care by the new io-domain driver if the dts
> also has correctly config.
>
> The io-domain driver is used for rk3568 firstly, because the IO will
> broken if the io voltage is not
>
> setting correctly(the IO looks working before it's broken).
>
> The cleanup is fine because there is no one to use it, and it would be
> better to make sure the
>
> IO voltage of  rk3308 VCCIO3 is also setting correctly for all boards.

All current supported rk3308 boards in mainline Linux and U-Boot
correctly signal IO voltage using GPIO0_A4 according to schematics.

rk3308-evb.dts and rk3308-roc-cc.dts is missing the io_domains node in
Linux, as I only added the node to boards I had (Rock Pi S and ROCK S0).

Is there something more you want me to do in regards to this patch?

Can possible add a second/send a separate patch that enable
ROCKCHIP_IODOMAIN for ROCKCHIP_RK3308, then someone just have to submit
io_domains patches to Linux and it will eventually be synced to U-Boot
during a future dts/upstream sync.

Regards,
Jonas

>
> Thanks,
> - Kever
>>
>> Cleanup by removing the unused rk_board_init() function and re-sort
>> included headers.
>>
>> Signed-off-by: Jonas Karlman<jonas@kwiboo.se>
>> ---
>> arch/arm/mach-rockchip/rk3308/rk3308.c | 69 +-------------------------
>> 1 file changed, 1 insertion(+), 68 deletions(-)
>>
>> diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c b/arch/arm/mach-rockchip/rk3308/rk3308.c
>> index 03d97e1d7460..6916f1a24441 100644
>> --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
>> +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
>> @@ -3,15 +3,12 @@
>> *Copyright (c) 2018 Rockchip Electronics Co., Ltd
>> */
>> #include <init.h>
>> -#include <malloc.h>
>> +#include <asm/armv8/mmu.h>
>> #include <asm/arch-rockchip/bootrom.h>
>> #include <asm/arch-rockchip/grf_rk3308.h>
>> #include <asm/arch-rockchip/hardware.h>
>> -#include <asm/gpio.h>
>> -#include <debug_uart.h>
>> #include <linux/bitops.h>
>>
>> -#include <asm/armv8/mmu.h>
>> static struct mm_region rk3308_mem_map[] = {
>> {
>> .virt = 0x0UL,
>> @@ -38,22 +35,6 @@ struct mm_region *mem_map = rk3308_mem_map;
>> #define SGRF_BASE 0xff2b0000
>>
>> enum {
>> - GPIO1C7_SHIFT = 8,
>> - GPIO1C7_MASK = GENMASK(11, 8),
>> - GPIO1C7_GPIO = 0,
>> - GPIO1C7_UART1_RTSN,
>> - GPIO1C7_UART2_TX_M0,
>> - GPIO1C7_SPI2_MOSI,
>> - GPIO1C7_JTAG_TMS,
>> -
>> - GPIO1C6_SHIFT = 4,
>> - GPIO1C6_MASK = GENMASK(7, 4),
>> - GPIO1C6_GPIO = 0,
>> - GPIO1C6_UART1_CTSN,
>> - GPIO1C6_UART2_RX_M0,
>> - GPIO1C6_SPI2_MISO,
>> - GPIO1C6_JTAG_TCLK,
>> -
>> GPIO4D3_SHIFT = 6,
>> GPIO4D3_MASK = GENMASK(7, 6),
>> GPIO4D3_GPIO = 0,
>> @@ -116,60 +97,12 @@ enum {
>> GPIO2A2_SEL_SRC_CTRL_SEL_PLUS = 1,
>> };
>>
>> -enum {
>> - IOVSEL3_CTRL_SHIFT = 8,
>> - IOVSEL3_CTRL_MASK = BIT(8),
>> - VCCIO3_SEL_BY_GPIO = 0,
>> - VCCIO3_SEL_BY_IOVSEL3,
>> -
>> - IOVSEL3_SHIFT = 3,
>> - IOVSEL3_MASK = BIT(3),
>> - VCCIO3_3V3 = 0,
>> - VCCIO3_1V8,
>> -};
>> -
>> -/*
>> - * The voltage of VCCIO3(which is the voltage domain of emmc/flash/sfc
>> - * interface) can indicated by GPIO0_A4 or io_vsel3. The SOC defaults
>> - * use GPIO0_A4 to indicate power supply voltage for VCCIO3 by hardware,
>> - * then we can switch to io_vsel3 after system power on, and release GPIO0_A4
>> - * for other usage.
>> - */
>> -
>> -#define GPIO0_A4 4
>> -
>> const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>> [BROM_BOOTSOURCE_EMMC] = "/mmc@ff490000",
>> [BROM_BOOTSOURCE_SPINOR] = "/spi@ff4c0000/flash@0",
>> [BROM_BOOTSOURCE_SD] = "/mmc@ff480000",
>> };
>>
>> -int rk_board_init(void)
>> -{
>> - static struct rk3308_grf * const grf = (void *)GRF_BASE;
>> - u32 val;
>> - int ret;
>> -
>> - ret = gpio_request(GPIO0_A4, "gpio0_a4");
>> - if (ret < 0) {
>> - printf("request for gpio0_a4 failed:%d\n", ret);
>> - return 0;
>> - }
>> -
>> - gpio_direction_input(GPIO0_A4);
>> -
>> - if (gpio_get_value(GPIO0_A4))
>> - val = VCCIO3_SEL_BY_IOVSEL3 << IOVSEL3_CTRL_SHIFT |
>> - VCCIO3_1V8 << IOVSEL3_SHIFT;
>> - else
>> - val = VCCIO3_SEL_BY_IOVSEL3 << IOVSEL3_CTRL_SHIFT |
>> - VCCIO3_3V3 << IOVSEL3_SHIFT;
>> - rk_clrsetreg(&grf->soc_con0, IOVSEL3_CTRL_MASK | IOVSEL3_MASK, val);
>> -
>> - gpio_free(GPIO0_A4);
>> - return 0;
>> -}
>> -
>> #ifdef CONFIG_DEBUG_UART_BOARD_INIT
>> __weak void board_debug_uart_init(void)
>> {