[PATCH 0/4] rockchip: Skip serial pinctrl at pre-reloc phase

[Quentin Schulz]

Hi Kever,

On 9/30/24 12:11 PM, Kever Yang wrote:
> Hi Quentin,
> 
> On 2024/8/12 18:44, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 8/5/24 2:22 PM, Jonas Karlman wrote:
>>> Hi Quentin,
>>>
>>> On 2024-08-05 12:23, Quentin Schulz wrote:
>>>> Hi Jonas,
>>>>
>>>> On 8/5/24 10:43 AM, Jonas Karlman wrote:
>>>>> UART pinctrl for serial is typically applied multiple times:
>>>>> - in external TPL or in TPL for DEBUG_UART in board_debug_uart_init()
>>>>> - in SPL for DEBUG_UART in board_debug_uart_init()
>>>>> - in SPL using pinctrl from DT
>>>>> - in pre-reloc phase using pinctrl from DT
>>>>> - after relocation using pinctrl from DT
>>>>>
>>>>> This series change bootph props for the UART pinctrl to not include 
>>>>> them
>>>>> in pre-reloc phase to save some boot time:
>>>>>
>>>>
>>>> NACK. I feel like this is a hack for vendor trees only.
>>>
>>> I disagree, this just relaxes the bootph props currently enforced by
>>> <soc>-u-boot.dtsi files. For boards that has special need they can
>>> still add bootph-all or the bootph-some-ram prop to restore prior
>>> behavior.
> 
> The pinctrl setting for the same module for multi times is obviously not 
> a good idea,
> 
> and for SPL, it used to work without pinctrl which is simple and fast 
> and enough for use.
> 
> Because in most case, people use the UART and storage with is the same 
> as the bootrom
> 
> used, so the IO has been already been initialized.
> 

Is the bootrom really initializing the UART? Are you sure you're not 
mixing bootrom and Rockchip's TPL blob? The latter, I know, the former, 
I'm surprised since it doesn't print anything as far as I am aware?

> This change works for most of the boards and for  different design can 
> customize for its
> 
> usage, them the patch is acceptable, this is different default setting 
> instead of hack.
> 

I disagree, this is working around the DT specs to make things faster by 
breaking some other usecases. I believe we shouldn't have to change the 
device tree when we change something in the defconfig for the board to 
still work.

I would be fine with a bunch of ifdefs in -u-boot.dtsi to have the 
appropriate bootph- properties guarded by the appropriate 
CONFIG_SPL_SERIAL/CONFIG_TPL_SERIAL, e.g. something like (not tested)

"""
// Optimize boot time by avoiding to reconfigure pinctrl already 
configured by earlier stages for console
#if IS_ENABLED(TPL_SERIAL)
     bootph-pre-sram;
#elif IS_ENABLED(SPL_SERIAL)
     bootph-pre-ram;
#else
     bootph-all;
#endif
"""

I also don't know what we should be doing with ROCKCHIP_EXTERNAL_TPL as 
we should probably have bootph-pre-ram/bootph-all set if that option is 
set, in case one uses the default blob from Rockchip but a different 
UART controller or mux so that they have console starting from upstream 
U-Boot at least.

What do you think?

Cheers,
Quentin