[PATCH v2 3/5] ram: rockchip: Add separate RAM_ROCKCHIP_DEBUG config for TPL/SPL

[Quentin Schulz]

Hi Kever, Lukasz,

On 9/10/24 5:17 PM, Łukasz Czechowski wrote:
> Hi Kever,
> Thanks for the review.
> This patch is indeed intended to disallow enabling RAM_ROCKCHIP_DEBUG
> when SILENT_CONSOLE is enabled and control what is printed on the
> debug console more independently. Patch was created in response to
> review of patchset v1, and implements change suggested by Quentin
> Schulz <quentin.schulz at cherry.de>. The additional complexity with new
> TPL/SPL symbols had been added because we couldn't simply add i.e.
> `depends on !TPL_SILENT_CONSOLE if TPL` and  `depends on
> !SPL_SILENT_CONSOLE if SPL`.
> @Quentin, is it fine with you that we can drop this change?
> 
> Best regards,
> Lukasz
> 

@Lukasz, please use interleaved style instead of top posting.

c.f. 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions 
and https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

You also may want to look into your mail client as Kever's answer is now 
at the same level as your original patch instead of a quote within a 
quote making it harder to know who wrote what and when.

> 
> wt., 10 wrz 2024 o 11:51 Kever Yang <kever.yang at rock-chips.com> napisał(a):
>>
>> Hi Lukasz,
>>
>> On 2024/9/4 00:38, Lukasz Czechowski wrote:
>>
>> Introduce new config symbols TPL_RAM_ROCKCHIP_DEBUG and
>> SPL_RAM_ROCKCHIP_DEBUG to allow for better dependencies control
>> of RAM driver debugging configuration.
>>
>> The RAM_ROCKCHIP_DEBUG should enough because this only happen when ram driver
>>
>> running the initialization, and this init process always only run only once for all SoCs.
>>
>> RAM_ROCKCHIP_DEBUG depends on DEBUG_UART is the correct and no need to add more
>>
>> other dependency.
>>
>> Add negative dependencies to TPL_SILENT_CONSOLE and
>> SPL_SILENT_CONSOLE, respectively.
>>
>> I believe this is he main target, you want to control the UART output by SILENT_CONTROL,
>>
>> but the RAM_DEBUG should follow the DEBUG_UART.
>>
>> So I think this patch is no need, please drop it.
>>

I have this gut feeling we shouldn't drop this patch but I couldn't find 
anything that would either confirm or contradict this gut feeling in the 
hour I looked.

I assume one wouldn't enable DEBUG_UART when one wants a silent console 
in the first stage (TPL or SPL, whatever that is) which is usually where 
DRAM happens (that may change with VBE I believe? but we're not entirely 
there yet). So I guess the dependency on DEBUG_UART is actually 
"stronger" than a dependency on TPL/SPL_SILENT_CONSOLE and therefore we 
should be able to drop this patch.

Cheers,
Quentin