[PATCH 2/2] bsps/riscv: add riscv/kendrytek210 BSP variant

[Alan Cudmore]

On Tue, Mar 14, 2023 at 11:35 PM Gedare Bloom <gedare at rtems.org> wrote:

> >> >> > diff --git a/bsps/riscv/riscv/start/bspstart.c
> b/bsps/riscv/riscv/start/bspstart.c
> >> >> > index 30d479ce88..a0b6e683f6 100644
> >> >> > --- a/bsps/riscv/riscv/start/bspstart.c
> >> >> > +++ b/bsps/riscv/riscv/start/bspstart.c
> >> >> > @@ -201,6 +201,14 @@ static uint32_t get_core_frequency(void)
> >> >> >      return fdt32_to_cpu(*val);
> >> >> >    }
> >> >> >  #endif
> >> >> > +
> >> >> > +#if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0
> >> >> > +  uint32_t cpu_clock;
> >> >> > +
> >> >> > +  cpu_clock = k210_get_frequency();
> >> >> > +  return cpu_clock;
> >> >> > +#endif
> >> >> > +
> >> >> >    return 0;
> >> >
> >> >
> >> > 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.
> >> > 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.
> >> >
> >> I would at  a minimum make it #else return 0
> >> to avoid having unreachable code in your build.
> >>
> >
> >  The "return 0" is right after my ifdef block. If I put #else return 0,
> then there will be two "return 0" statements.
> > If none of the variant options are defined, then the entire routine
> defaults to "return 0" at the bottom.
> > Should I eliminate that and put the else clauses for the FRDME310ARTY
> and MPFS variants too?
>
> I meant to put the 'return 0;' in an #else
> That will leave the tail of the function with just one return
> statement, dependent on the CPP macro conditional.
> #if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0
>   uint32_t cpu_clock;
>
>   cpu_clock = k210_get_frequency();
>   return cpu_clock;
> #else
>   return 0;
> #endif
>
> This will avoid having a dead code issue. The other blocks above don't
> need to be modified.
>
> 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.
I'll send a v2 patch set. I also removed some whitespace warnings that
occur when you apply the patches.
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.
Thanks,
Alan


> > Thanks,
> > Alan
> >
> >>
> >>
> >> >
> >> >> This code is unreachable if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0.
> >> >>
> >> >> >  }
> >> >> >
> >> >> > @@ -215,6 +223,40 @@ uint32_t bsp_fdt_map_intr(const uint32_t
> *intr, size_t icells)
> >> >> >    return RISCV_INTERRUPT_VECTOR_EXTERNAL(intr[0]);
> >> >> >  }
> >> >> >
> >> >> > +#if RISCV_ENABLE_KENDRYTE_K210_SUPPORT != 0
> >> >> > +uint32_t k210_get_frequency(void)
> >> >> > +{
> >> >> > +  k210_sysctl_t *sysctl = (k210_sysctl_t *)K210_SYSCTL_BASE;
> >> >> > +  uint32_t cpu_clock = 0;
> >> >> > +  uint32_t clk_freq;
> >> >> > +  uint32_t pll0, nr, nf, od;
> >> >> > +  uint32_t node;
> >> >> > +  const char *fdt;
> >> >> > +  const fdt32_t *val;
> >> >> > +  int len;
> >> >> > +
> >> >> > +  fdt = bsp_fdt_get();
> >> >> > +  node = fdt_node_offset_by_compatible(fdt, -1,"fixed-clock");
> >> >> > +  val = fdt_getprop(fdt, node, "clock-frequency", &len);
> >> >> > +  if (val != NULL && len == 4) {
> >> >> > +    clk_freq = fdt32_to_cpu(*val);
> >> >> > +
> >> >> > +    if (CLKSEL0_ACLK_SEL(sysctl->clk_sel0) == 1) {
> >> >> > +       /* PLL0 selected */
> >> >> > +       pll0 = sysctl->pll0;
> >> >> > +       nr = PLL_CLK_R(pll0) + 1;
> >> >> > +       nf = PLL_CLK_F(pll0) + 1;
> >> >> > +       od = PLL_CLK_OD(pll0) + 1;
> >> >> > +       cpu_clock = (clk_freq / nr * nf / od)/2;
> >> >> > +    } else {
> >> >> > +       /* OSC selected */
> >> >> > +       cpu_clock = clk_freq;
> >> >> > +    }
> >> >> > +  }
> >> >> > +  return cpu_clock;
> >> >> > +}
> >> >> > +#endif
> >> >> > +
> >> >> >  void bsp_start(void)
> >> >> >  {
> >> >> >    riscv_find_harts();
> >> >> > --
> >> >> > 2.25.1
> >> >> >
> >> >> > _______________________________________________
> >> >> > devel mailing list
> >> >> > devel at rtems.org
> >> >> > http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20230315/5c07e0f1/attachment.htm>