On 2024-09-26 23:16, Paul Kocialkowski wrote:
Le Thu 26 Sep 24, 23:04, Dragan Simic a écrit :
On 2024-09-26 22:51, Paul Kocialkowski wrote:
> Le Thu 26 Sep 24, 22:17, Dragan Simic a écrit :
> > On 2024-09-26 20:31, Paul Kocialkowski wrote:
> > > From: Paul Kocialkowski <cont...@paulk.fr>
> > >
> > > Printing debug details about DRAM is not useful in regular use and
> > > adds visual pollution to the log. Disable it by default.
> >
> > With all the respect, I disagree with disabling this by default.
> > This prints just a couple of lines that can actually be very helpful
> > when figuring out what's going on in case of some DRAM-related issues
> > on random devices in the field.
>
> Well this rationale could apply to lots of things and we generally don't
> print debug info about anything else by default.
I'd rather see these messages as some kind of additional verbosity,
rather than some true debugging messages for the DRAM init. It's just
a couple of additional lines printed on the console, in the end.
It's mostly the fact that it's platform-specific and makes about no
sense
to a non-developer user that makes me think it's not welcome by
default.
It's really printing internal variables, not providing user-readable
information
that can be useful outside of the scope of development.
Well, each and every U-Boot build is platform- and device-specific, so
I see no problems with some of the messages produced by default being
platform- or device-specific.
In practice, anything that U-Boot prints on the console has little to no
meaning to the vast majority of people who actually happen to see and
register those messages. As a result, those messages are mostly useful
to the developers only.
> Maybe DRAM is more likely to be a source of issues than other hardware
> aspects
> that are maybe more stable, but I don't see what would prevent
> rebuilding a
> u-boot binary with debug enabled. If the DRAM config needs tweaking it
> will be
> necessary to rebuild a binary anyway.
In theory, rebuilding a U-Boot image and reinstalling it is rather
easy.
In practice, it's hardly doable on random devices in the field, and
it's
sometimes virtually impossible. It's simply that not every end user
is
willing or capable of doing things like that.
Well that is more or less what makes me think this is not welcome by
default:
a non-developer user would not be able to do anything useful with this
information, since they won't be able to rebuild a binary to change the
DRAM
config and solve any DRAM-related issue.
A non-developer may be able to forward the messages to a developer, who
in turn may be able to use them to roughly figure out what's going on.
Just knowing this information "on the field" won't help solve any issue
if
there is no possibility to undergo development and build a binary with
a
modified DRAM config.
In what scenario exactly do you think this would be valuable
information to
a non-developer?
I'm actually speaking from experience. Those messages have already
proven to be useful in a few cases that I happened to witness.
> > > Signed-off-by: Paul Kocialkowski <cont...@paulk.fr>
> > > ---
> > > configs/anbernic-rgxx3-rk3566_defconfig | 1 -
> > > configs/neu2-io-rv1126_defconfig | 1 -
> > > configs/roc-pc-mezzanine-rk3399_defconfig | 1 -
> > > configs/roc-pc-rk3399_defconfig | 1 -
> > > configs/rock-pi-n10-rk3399pro_defconfig | 1 -
> > > configs/rock-pi-n8-rk3288_defconfig | 1 -
> > > configs/sonoff-ihost-rv1126_defconfig | 1 -
> > > drivers/ram/rockchip/Kconfig | 1 -
> > > 8 files changed, 8 deletions(-)
> > >
> > > diff --git a/configs/anbernic-rgxx3-rk3566_defconfig
> > > b/configs/anbernic-rgxx3-rk3566_defconfig
> > > index a03509bf4671..5c074cffeb44 100644
> > > --- a/configs/anbernic-rgxx3-rk3566_defconfig
> > > +++ b/configs/anbernic-rgxx3-rk3566_defconfig
> > > @@ -67,7 +67,6 @@ CONFIG_SPL_DM_REGULATOR_FIXED=y
> > > CONFIG_REGULATOR_RK8XX=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > CONFIG_SPL_RAM=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > # CONFIG_RNG_SMCCC_TRNG is not set
> > > CONFIG_BAUDRATE=1500000
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > diff --git a/configs/neu2-io-rv1126_defconfig
> > > b/configs/neu2-io-rv1126_defconfig
> > > index 2a4c9b45a04f..84e4465f2c5f 100644
> > > --- a/configs/neu2-io-rv1126_defconfig
> > > +++ b/configs/neu2-io-rv1126_defconfig
> > > @@ -45,7 +45,6 @@ CONFIG_MMC_DW=y
> > > CONFIG_MMC_DW_ROCKCHIP=y
> > > CONFIG_REGULATOR_PWM=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > CONFIG_BAUDRATE=1500000
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > CONFIG_SYSRESET=y
> > > diff --git a/configs/roc-pc-mezzanine-rk3399_defconfig
> > > b/configs/roc-pc-mezzanine-rk3399_defconfig
> > > index a57899bfdfa0..b4041902b381 100644
> > > --- a/configs/roc-pc-mezzanine-rk3399_defconfig
> > > +++ b/configs/roc-pc-mezzanine-rk3399_defconfig
> > > @@ -65,7 +65,6 @@ CONFIG_REGULATOR_PWM=y
> > > CONFIG_SPL_DM_REGULATOR_FIXED=y
> > > CONFIG_REGULATOR_RK8XX=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > CONFIG_RAM_ROCKCHIP_LPDDR4=y
> > > CONFIG_BAUDRATE=1500000
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > diff --git a/configs/roc-pc-rk3399_defconfig
> > > b/configs/roc-pc-rk3399_defconfig
> > > index b45f0e0a8994..922f67320c20 100644
> > > --- a/configs/roc-pc-rk3399_defconfig
> > > +++ b/configs/roc-pc-rk3399_defconfig
> > > @@ -62,7 +62,6 @@ CONFIG_REGULATOR_PWM=y
> > > CONFIG_SPL_DM_REGULATOR_FIXED=y
> > > CONFIG_REGULATOR_RK8XX=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > CONFIG_RAM_ROCKCHIP_LPDDR4=y
> > > CONFIG_BAUDRATE=1500000
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > diff --git a/configs/rock-pi-n10-rk3399pro_defconfig
> > > b/configs/rock-pi-n10-rk3399pro_defconfig
> > > index ec995a54a0ee..17fe939ec989 100644
> > > --- a/configs/rock-pi-n10-rk3399pro_defconfig
> > > +++ b/configs/rock-pi-n10-rk3399pro_defconfig
> > > @@ -51,7 +51,6 @@ CONFIG_PHY_ROCKCHIP_TYPEC=y
> > > CONFIG_PMIC_RK8XX=y
> > > CONFIG_REGULATOR_RK8XX=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > CONFIG_BAUDRATE=1500000
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > CONFIG_SYS_NS16550_MEM32=y
> > > diff --git a/configs/rock-pi-n8-rk3288_defconfig
> > > b/configs/rock-pi-n8-rk3288_defconfig
> > > index 4c09b9137ef8..af0fa8879421 100644
> > > --- a/configs/rock-pi-n8-rk3288_defconfig
> > > +++ b/configs/rock-pi-n8-rk3288_defconfig
> > > @@ -73,7 +73,6 @@ CONFIG_REGULATOR_RK8XX=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > CONFIG_RAM=y
> > > CONFIG_SPL_RAM=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > CONFIG_SYS_NS16550_MEM32=y
> > > CONFIG_SYSRESET=y
> > > diff --git a/configs/sonoff-ihost-rv1126_defconfig
> > > b/configs/sonoff-ihost-rv1126_defconfig
> > > index 4890644c7e6f..739adb49ce93 100644
> > > --- a/configs/sonoff-ihost-rv1126_defconfig
> > > +++ b/configs/sonoff-ihost-rv1126_defconfig
> > > @@ -46,7 +46,6 @@ CONFIG_MMC_DW=y
> > > CONFIG_MMC_DW_ROCKCHIP=y
> > > CONFIG_REGULATOR_PWM=y
> > > CONFIG_PWM_ROCKCHIP=y
> > > -# CONFIG_RAM_ROCKCHIP_DEBUG is not set
> > > CONFIG_BAUDRATE=1500000
> > > CONFIG_DEBUG_UART_SHIFT=2
> > > CONFIG_SYSRESET=y
> > > diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig
> > > index 67c63ecba047..e030c982eccb 100644
> > > --- a/drivers/ram/rockchip/Kconfig
> > > +++ b/drivers/ram/rockchip/Kconfig
> > > @@ -15,7 +15,6 @@ if RAM_ROCKCHIP
> > >
> > > config RAM_ROCKCHIP_DEBUG
> > > bool "Rockchip ram drivers debugging"
> > > - default y
> > > help
> > > This enables debugging ram driver API's for the platforms
> > > based on Rockchip SoCs.
