<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 14, 2023 at 11:35 PM Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">>> >> > diff --git a/bsps/riscv/riscv/start/bspstart.c b/bsps/riscv/riscv/start/bspstart.c<br>
>> >> > index 30d479ce88..a0b6e683f6 100644<br>
>> >> > --- a/bsps/riscv/riscv/start/bspstart.c<br>
>> >> > +++ b/bsps/riscv/riscv/start/bspstart.c<br>
>> >> > @@ -201,6 +201,14 @@ static uint32_t get_core_frequency(void)<br>
>> >> > return fdt32_to_cpu(*val);<br>
>> >> > }<br>
>> >> > #endif<br>
>> >> > +<br>
>> >> > +#if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0<br>
>> >> > + uint32_t cpu_clock;<br>
>> >> > +<br>
>> >> > + cpu_clock = k210_get_frequency();<br>
>> >> > + return cpu_clock;<br>
>> >> > +#endif<br>
>> >> > +<br>
>> >> > return 0;<br>
>> ><br>
>> ><br>
>> > When you choose the kendrtyek210 BSP variant, the RISCV_ENABLE_KENDRYTE_K210_SUPPORT is set to true enabling the code that is needed for frequency calculation in this file. I tried to follow the same pattern for the MPFS and FRDME310ARTY variants here.<br>
>> > The K210, FRME310ARTY, and MPFS options could probably use refactoring, but I was reluctant to change existing code for the MPFS and 310ARTY since I do not have a way of testing them.<br>
>> ><br>
>> I would at a minimum make it #else return 0<br>
>> to avoid having unreachable code in your build.<br>
>><br>
><br>
> The "return 0" is right after my ifdef block. If I put #else return 0, then there will be two "return 0" statements.<br>
> If none of the variant options are defined, then the entire routine defaults to "return 0" at the bottom.<br>
> Should I eliminate that and put the else clauses for the FRDME310ARTY and MPFS variants too?<br>
<br>
I meant to put the 'return 0;' in an #else<br>
That will leave the tail of the function with just one return<br>
statement, dependent on the CPP macro conditional.<br>
#if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0<br>
uint32_t cpu_clock;<br>
<br>
cpu_clock = k210_get_frequency();<br>
return cpu_clock;<br>
#else<br>
return 0;<br>
#endif<br>
<br>
This will avoid having a dead code issue. The other blocks above don't<br>
need to be modified.<br>
<br></blockquote><div>I understand now. The 310ARTY or MPFS could fall through to the 'return 0', but the k210 code will always return leaving the extra 'return 0' as dead code.</div><div>I'll send a v2 patch set. I also removed some whitespace warnings that occur when you apply the patches.</div><div>In addition to a few whitespace warnings in the code I added, the rtems-bin2c tool leaves a space at the end of each line in the array, causing git to complain about the extra whitespace. If you think that should be fixed we could put in a ticket for the tool. That could be an easy contribution for a student.</div><div>Thanks,</div><div>Alan </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Thanks,<br>
> Alan<br>
><br>
>><br>
>><br>
>> ><br>
>> >> This code is unreachable if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0.<br>
>> >><br>
>> >> > }<br>
>> >> ><br>
>> >> > @@ -215,6 +223,40 @@ uint32_t bsp_fdt_map_intr(const uint32_t *intr, size_t icells)<br>
>> >> > return RISCV_INTERRUPT_VECTOR_EXTERNAL(intr[0]);<br>
>> >> > }<br>
>> >> ><br>
>> >> > +#if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0<br>
>> >> > +uint32_t k210_get_frequency(void)<br>
>> >> > +{<br>
>> >> > + k210_sysctl_t *sysctl = (k210_sysctl_t *)K210_SYSCTL_BASE;<br>
>> >> > + uint32_t cpu_clock = 0;<br>
>> >> > + uint32_t clk_freq;<br>
>> >> > + uint32_t pll0, nr, nf, od;<br>
>> >> > + uint32_t node;<br>
>> >> > + const char *fdt;<br>
>> >> > + const fdt32_t *val;<br>
>> >> > + int len;<br>
>> >> > +<br>
>> >> > + fdt = bsp_fdt_get();<br>
>> >> > + node = fdt_node_offset_by_compatible(fdt, -1,"fixed-clock");<br>
>> >> > + val = fdt_getprop(fdt, node, "clock-frequency", &len);<br>
>> >> > + if (val != NULL && len == 4) {<br>
>> >> > + clk_freq = fdt32_to_cpu(*val);<br>
>> >> > +<br>
>> >> > + if (CLKSEL0_ACLK_SEL(sysctl->clk_sel0) == 1) {<br>
>> >> > + /* PLL0 selected */<br>
>> >> > + pll0 = sysctl->pll0;<br>
>> >> > + nr = PLL_CLK_R(pll0) + 1;<br>
>> >> > + nf = PLL_CLK_F(pll0) + 1;<br>
>> >> > + od = PLL_CLK_OD(pll0) + 1;<br>
>> >> > + cpu_clock = (clk_freq / nr * nf / od)/2;<br>
>> >> > + } else {<br>
>> >> > + /* OSC selected */<br>
>> >> > + cpu_clock = clk_freq;<br>
>> >> > + }<br>
>> >> > + }<br>
>> >> > + return cpu_clock;<br>
>> >> > +}<br>
>> >> > +#endif<br>
>> >> > +<br>
>> >> > void bsp_start(void)<br>
>> >> > {<br>
>> >> > riscv_find_harts();<br>
>> >> > --<br>
>> >> > 2.25.1<br>
>> >> ><br>
>> >> > _______________________________________________<br>
>> >> > devel mailing list<br>
>> >> > <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
>> >> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div>