[U-Boot] [PATCH v2 03/10] rockchip: rk3036: add board_debug_uart_init()

[Kever Yang]

Hi Jack,


On 03/29/2019 06:10 PM, Jack Mitchell wrote:
>
> On 29/03/2019 01:09, Kever Yang wrote:
>> Use board_debug_uart_init() for UART iomux init instead of
>> do it in board_init_f, and move the function to soc file so
>> that we can find all the soc/board setting in soc file and
>> use a common board file.
>>
>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>> ---
>>
>>  arch/arm/mach-rockchip/rk3036-board-spl.c | 20 +-----------
>>  arch/arm/mach-rockchip/rk3036/Makefile    |  1 +
>>  arch/arm/mach-rockchip/rk3036/rk3036.c    | 39 +++++++++++++++++++++++
>>  3 files changed, 41 insertions(+), 19 deletions(-)
>>  create mode 100644 arch/arm/mach-rockchip/rk3036/rk3036.c
>>
>> diff --git a/arch/arm/mach-rockchip/rk3036-board-spl.c b/arch/arm/mach-rockchip/rk3036-board-spl.c
>> index b897ffeae2..110d06dba5 100644
>> --- a/arch/arm/mach-rockchip/rk3036-board-spl.c
>> +++ b/arch/arm/mach-rockchip/rk3036-board-spl.c
>> @@ -7,30 +7,12 @@
>>  #include <debug_uart.h>
>>  #include <asm/io.h>
>>  #include <asm/arch-rockchip/bootrom.h>
>> -#include <asm/arch-rockchip/grf_rk3036.h>
>> -#include <asm/arch-rockchip/hardware.h>
>>  #include <asm/arch-rockchip/sdram_rk3036.h>
>>  #include <asm/arch-rockchip/timer.h>
>> -#include <asm/arch-rockchip/uart.h>
>> -
>> -#define GRF_BASE	0x20008000
>> -
>> -#define DEBUG_UART_BASE	0x20068000
>>  
>>  void board_init_f(ulong dummy)
>>  {
>> -#ifdef EARLY_DEBUG
>> -	struct rk3036_grf * const grf = (void *)GRF_BASE;
>> -	/*
>> -	 * NOTE: sd card and debug uart use same iomux in rk3036,
>> -	 * so if you enable uart,
>> -	 * you can not boot from sdcard
>> -	 */
>> -	rk_clrsetreg(&grf->gpio1c_iomux,
>> -		     GPIO1C3_MASK << GPIO1C3_SHIFT |
>> -		     GPIO1C2_MASK << GPIO1C2_SHIFT,
>> -		     GPIO1C3_UART2_SOUT << GPIO1C3_SHIFT |
>> -		     GPIO1C2_UART2_SIN << GPIO1C2_SHIFT);
>> +#ifdef CONFIG_DEBUG_UART
>>  	debug_uart_init();
>>  #endif
>>  	rockchip_timer_init();
>> diff --git a/arch/arm/mach-rockchip/rk3036/Makefile b/arch/arm/mach-rockchip/rk3036/Makefile
>> index 20d28f7c21..299fc50635 100644
>> --- a/arch/arm/mach-rockchip/rk3036/Makefile
>> +++ b/arch/arm/mach-rockchip/rk3036/Makefile
>> @@ -10,4 +10,5 @@ ifndef CONFIG_SPL_BUILD
>>  obj-y += syscon_rk3036.o
>>  endif
>>  
>> +obj-y += rk3036.o
>>  obj-y += sdram_rk3036.o
>> diff --git a/arch/arm/mach-rockchip/rk3036/rk3036.c b/arch/arm/mach-rockchip/rk3036/rk3036.c
>> new file mode 100644
>> index 0000000000..32426ac840
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/rk3036/rk3036.c
>> @@ -0,0 +1,39 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2019 Rockchip Electronics Co., Ltd
>> + */
>> +#include <asm/io.h>
>> +#include <asm/arch-rockchip/grf_rk3036.h>
>> +#include <asm/arch-rockchip/hardware.h>
>> +
>> +#ifdef CONFIG_DEBUG_UART_BOARD_INIT
>> +void board_debug_uart_init(void)
>> +{
>> +#define GRF_BASE	0x20008000
>> +	struct rk3036_grf * const grf = (void *)GRF_BASE;
>> +	enum {
>> +		GPIO1C3_SHIFT		= 6,
>> +		GPIO1C3_MASK		= 3 << GPIO1C3_SHIFT,
>> +		GPIO1C3_GPIO		= 0,
>> +		GPIO1C3_MMC0_D1,
>> +		GPIO1C3_UART2_SOUT,
>> +
>> +		GPIO1C2_SHIFT		= 4,
>> +		GPIO1C2_MASK		= 3 << GPIO1C2_SHIFT,
>> +		GPIO1C2_GPIO		= 0,
>> +		GPIO1C2_MMC0_D0,
>> +		GPIO1C2_UART2_SIN,
>> +	};
> Would it be better to put this enum in the grf_rk3036.h header file as
> is done on the rk3288 to keep some consistency and have a central place
> where these defines are kept? Same for the other patches where the enums
> are defined in place.

Thanks for your comment, but this kind of style is recommend by the
maintainer, so maybe the rk3288 is the one need to update.

Thanks,
- Kever
>> +	/*
>> +	 * NOTE: sd card and debug uart use same iomux in rk3036,
>> +	 * so if you enable uart,
>> +	 * you can not boot from sdcard
>> +	 */
>> +	rk_clrsetreg(&grf->gpio1c_iomux,
>> +		     GPIO1C3_MASK << GPIO1C3_SHIFT |
>> +		     GPIO1C2_MASK << GPIO1C2_SHIFT,
>> +		     GPIO1C3_UART2_SOUT << GPIO1C3_SHIFT |
>> +		     GPIO1C2_UART2_SIN << GPIO1C2_SHIFT);
>> +}
>> +#endif
>> +
>>