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@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-t... 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@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