warnings, fixes, cleanups #32
Conversation
This is very cheesy and frankly, unnecessary.
konradybcio
force-pushed
the
topic/brrrr
branch
from
b13b6b0
to
9d01d16
Compare
| @@ -207,32 +207,6 @@ static const struct debugcc_platform *find_platform(const char *name) | |||
| return NULL; | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
I can not say that it is unnecessary. I frequenty run it as platform-debugcc instead of debugcc -p platform.
BTW: is your commit not signed-of, or is it a Github hiding that bit of the commit message?
There was a problem hiding this comment.
Even better would be if debugcc could detect the platform and not have us accidentally map and read/write the wrong memory region for a different platform (resulting in an insta-crash usually).
I tend to do this a lot thanks to shell history.
There was a problem hiding this comment.
@lumag xkcd 1172 😛
@MarijnS95 should be very doable.. we can probably just read the compatible from /sys/firmware/devicetree
| @@ -192,7 +192,7 @@ static void measure(const struct measure_clk *clk) | |||
| return; | |||
| } | |||
|
|
|||
| printf("%50s: %fMHz (%ldHz)\n", clk->name, clk_rate / 1000000.0, clk_rate); | |||
| printf("%50s: %fMHz (%luHz)\n", clk->name, clk_rate / 1000000.0, clk_rate); | |||
| @@ -144,317 +144,317 @@ static struct debug_mux video_cc = { | |||
|
|
|||
| static struct measure_clk sm8650_clocks[] = { | |||
| /* GCC entries */ | |||
| { "gcc_aggre_noc_pcie_axi_clk", &gcc.mux, 0x3f }, | |||
There was a problem hiding this comment.
I usually like C99. However in this case struct initialisation becomes less readable.
There was a problem hiding this comment.
Well either that or we have to fill out all magic fields to make the "struct is only partially initialized" warning (forgot the name) satisfied
| { .name = "l2_m_clk", .clk_mux = &cpul2_mux, .mux = 0x2, 8 }, | ||
| { .name = "krait0_m_clk", .clk_mux = &cpul2_mux, .mux = 0x0, 8 }, | ||
| { .name = "krait1_m_clk", .clk_mux = &cpul2_mux, .mux = 0x1, 8 }, |
There was a problem hiding this comment.
think the last value is missing?
There was a problem hiding this comment.
It works out by total luck but you're right!
|
@konradybcio MAYBE to save reliability and mute the warning... we define a macro that define the full struct with the entry declared? And also add macro variants for the additional values? (like the case of ipq806x with the divider?) |
For the record we dropped exactly this pattern in favour of C99 struct initialization in the DPU catalog, to make entries more readable by having explicit names. |
|
@MarijnS95 ... And turning structs into multiline constructions, one field per line. I'm fine if that works approach is implemented here. |
|
@konradybcio do you have plans to finish this pull request? |
sm8250-debugccand alike