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.sch...@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.y...@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
