Hi Simon,
On 6/6/24 5:04 PM, Simon Glass wrote:
HI Quentin,
On Wed, 5 Jun 2024 at 02:36, Quentin Schulz <quentin.sch...@cherry.de> wrote:
Hi Simon,
On 6/5/24 5:25 AM, Simon Glass wrote:
Add better logging for power init so that CONFIG_LOG_ERROR_RETURN can
be enabled.
Signed-off-by: Simon Glass <s...@chromium.org>
---
board/google/veyron/veyron.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/board/google/veyron/veyron.c b/board/google/veyron/veyron.c
index 32dbcdc4d10..23fe8bf088c 100644
--- a/board/google/veyron/veyron.c
+++ b/board/google/veyron/veyron.c
@@ -29,44 +29,41 @@ static int veyron_init(void)
int ret;
ret = regulator_get_by_platname("vdd_arm", &dev);
- if (ret) {
- debug("Cannot set regulator name\n");
- return ret;
- }
+ if (ret)
+ return log_msg_ret("vdd", ret);
Those log messages aren't for code in SPL as far as I could tell, is
there any reason to make them that small/cryptic?
Oh yes, it happens early in U-Boot proper before any messages which is
probably what confused me.
Re size, they just need to support searching the code base. Long
strings mean that many boards fail to boot / hit their limits when
CONFIG_LOG_ERROR_RETURN is enabled. So I try to keep them to 3
characters.
git grep vcc/vdd is going to return a lot of matches :)
This is for one board, so there isn't much reason to argue about this,
so fine :)
/* Slowly raise to max CPU voltage to prevent overshoot */
ret = regulator_set_value(dev, 1200000);
if (ret)
- return ret;
+ return log_msg_ret("s12", ret);
udelay(175); /* Must wait for voltage to stabilize, 2mV/us */
ret = regulator_set_value(dev, 1400000);
if (ret)
- return ret;
+ return log_msg_ret("s14", ret);
udelay(100); /* Must wait for voltage to stabilize, 2mV/us */
ret = rockchip_get_clk(&clk.dev);
if (ret)
- return ret;
+ return log_msg_ret("clk", ret);
clk.id = PLL_APLL;
ret = clk_set_rate(&clk, 1800000000);
if (IS_ERR_VALUE(ret))
- return ret;
+ return log_msg_ret("s18", ret);
ret = regulator_get_by_platname("vcc33_sd", &dev);
if (ret) {
debug("Cannot get regulator name\n");
- return ret;
+ if (ret)
+ return log_msg_ret("vcc", ret);
I think you can just merge the debug and log_msg_ret here?
They are actually different. The debug message is how I actually found
this problem.
I meant:
"""
if (ret) {
debug("Cannot get regulator name\n");
if (ret)
return log_msg_ret("vcc", ret);
}
"""
Nothing changes ret before the third line, so at the very least we could
have:
"""
if (ret) {
debug("Cannot get regulator name\n");
return log_msg_ret("vcc", ret);
}
"""
But i was also suggesting we merge this into:
"""
if (ret)
return log_msg_ret("Cannot get vcc regulator name", ret);
"""
But since you seem to want to keep only a few characters, then just
removing the debug entirely? Or what's the plan with it? (I see
log_msg_ret as a debug message, but I'm probably wrong here?).
Cheers,
Quentin
