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

[Gedare Bloom]

>> >> > 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.

> 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