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

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

[Jonas Karlman]

Nothing is calling the function rk_board_init() and the io-domain driver
can handle the functions intended purpose based on information from DT.

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)
 {
-- 
2.46.2

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

[Quentin Schulz]

Hi Jonas,

On 11/2/24 9:45 PM, 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.
> 
> Cleanup by removing the unused rk_board_init() function and re-sort
> included headers.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

git log -p -S rk_board_init

only returns one match and it's the introducing commit in which this 
function seemingly is never called.

I'm really wondering why this wasn't shown as a warning by GCC?

Since this is the IO domain for storage medium on RK3308, shouldn't we 
make sure the IO domain is enabled in SPL for fallback mechanism (i.e. 
proper isn't found on the same storage medium as the one used for 
loading SPL)?

The IODOMAIN isn't even enabled in proper for the EVB and the ROC-CC 
board. And there isn't an IODOMAIN symbol for SPL in Kconfig AFAICT?

Not that this commit would change anything, so:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

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

[Jonas Karlman]

Hi Quentin,

On 2024-11-06 14:49, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 11/2/24 9:45 PM, 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.
>>
>> Cleanup by removing the unused rk_board_init() function and re-sort
>> included headers.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> 
> git log -p -S rk_board_init
> 
> only returns one match and it's the introducing commit in which this 
> function seemingly is never called.
> 
> I'm really wondering why this wasn't shown as a warning by GCC?

Building with W=1 at least show one warning (and a lot of others):

  warning: no previous prototype for ‘rk_board_init’ [-Wmissing-prototypes]

> 
> Since this is the IO domain for storage medium on RK3308, shouldn't we 
> make sure the IO domain is enabled in SPL for fallback mechanism (i.e. 
> proper isn't found on the same storage medium as the one used for 
> loading SPL)?

I took a new look at schematics of supported boards and all seem to
signal correct VCCIO3 voltage using GPIO0_A4 (0: 3v3, 1: 1v8).

Also re-tested emmc/sd-nand and sd-card cross-read in SPL and there was
no issues on my Rock Pi S and ROCK S0 boards, so current reset values
to use GPIO0_A4 should work until io-domain driver later decide change
to use io_vsel3.

> 
> The IODOMAIN isn't even enabled in proper for the EVB and the ROC-CC 
> board. And there isn't an IODOMAIN symbol for SPL in Kconfig AFAICT?

Originally I probably only wanted to enable the io-domain driver on
boards that I could runtime test. Also the primary use for Rock Pi S and
ROCK S0 was to configure correct io voltage for Wi-Fi.

For rk3308 U-Boot IODOMAIN=y may not be fully necessary, at least now
that Linux io-domain driver also has support for rk3308.

For now, I do not think any board need to include io-domain driver in
SPL, could be changed in a future series if/when that is needed :-)

Regards,
Jonas

> 
> Not that this commit would change anything, so:
> 
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> 
> Thanks!
> Quentin

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

[Kever Yang]

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.

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)
>   {

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

[Jonas Karlman]

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)
>>   {

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

[Kever Yang]

Hi Jonas,

On 2024/11/11 17:41, Jonas Karlman wrote:
> 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?
I just want to make sure the io-domain driver also works correctly for 
rk3308, because it's
first used for rk3568 and potentially not correctly adapt for rk3308 in 
driver side or dts side.
For this patch itself, it's OK to apply  because the board does not 
really use it.

In some board design, theGPIO0_A4 in rk3308 board is used to signal IO 
voltage at the beginning,
and then this IO can be release to use for other purpose, that's why we 
have to init this in U-Boot
and out of the io-domain frame work which the GPIO0_A4 in dts can only 
used for only one purpose.

Thanks,
- Kever
>
> 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)
>>>    {
>

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

[Kever Yang]

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.
>
> Cleanup by removing the unused rk_board_init() function and re-sort
> included headers.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   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)
>   {