cpu/fe310: several cleanup in implementation

[aabadie]

Contribution description

This PR tries to improve the RISCV FE310 cpu implementation:

• it completely rework the clock initialization to use the PLL
• it drops some spurious board_clock_init functions used in the board initialization that adds a 2s delay during startup
• it splits the code in cpu.c in irq_arch.c, thread_arch.c and clock.c
• it reorganize the cpu_init function: nano stubs are now initialized before cpu peripherals, like other arch
• it also improves the different macro definitions at board level: xtimer macro are moved (and cleaned up) to board.h, clock configuration are set in periph_conf.h
• there's also some cleanup made in header includes

Now most of the tests run fine on the board but the on-board flasher is not very reliable so it cannot be really automatized.

The clock initialization only supports PLL and PLL bypass. Other clock source could be configured but I haven't looked at it. I have to say that I'm not 100% sure of the approach here, especially with low-power. Any help here would be appreciated.

I know the PR is quite large and will be difficult to test and review.

Follow-up work still needed:

• factorize common code in hifive1 boards. They are almost the same in fact
• improve clock initialization (with ROSC and XOSC clock source)
• implement thread isr stack management

This PR is only tested on hifive1b.

Testing procedure

Use compile_and_test_for_board.py on hifive1b: most of the test should pass. But sometimes they break because the flasher fail. Using an external JLink adapter could help for more reliable results. Some tests are failing but are not related to this PR (tests/ps_schedstatistics, because the xtimer low-level timer is clocked at 32768Hz)

Issues/PRs references

This PR is based on #12917

[kaspar030]

Use compile_and_test_for_board.py on hifive1b: most of the test should pass. But sometimes they break because the flasher fail. Using an external JLink adapter could help for more reliable results.

I had many problems with the onboard jlink until I upgraded JLink to the latest version (v6.56b). That in turn updated the onboard JLink firmware. Since then flashing is super reliable. Maybe that helps?

[kaspar030]

@kenrabold this might be interesting to you

[aabadie]

Maybe that helps?

It helps a lot. Thanks!

[kenrabold]

Overall I really like the changes. Much cleaner and I like the option for various clock selections. I had a couple questions about the SPI flash config, but overall looks good to me. I will try out the PR on both of the HiFive1 and HiFive1b boards that I have.

[aabadie]

I had a couple questions about the SPI flash config

Note that I only tested with hifive1b and it's working well without the code related to quad spi. It's still unclear to me why this quad SPI mode was needed. Can you point to some documentation @kenrabold ?

[kenrabold]

There are two reasons for wanting QuadIO mode:

1. The default setting is single line SPI mode which means all the data is serialized vs Dual or Quad mode where 2 or 4 lines in parallel are used. Speed up in data transfers because there are fewer clock cycles per byte transfered
2. In Quad mode you can crank up the SPI clk frequency to 133MHz. The default setting for SPI sckdiv = 3, which means you are dividing the core clock by 2(div+1) = 8 to get the SPI clock.
   Here is the SPI flash datasheet: http://www.issi.com/WW/pdf/25LP-WP032D.pdf

[aabadie]

@kenrabold, I restored the flash initialization function but it's still unclear to me what is the related between the CPU frequency, the SPI clock divider and the maximum flash frequency.

In the current state of this PR, the default CPU clock is 48MHz. When using the HFROSC, it's about 36MHz and when using the HFXOSC without PLL, 16MHz. I'll compare with the core frequency configured in master.

[kenrabold]

Why are you running the CPU at such a slow speed? I have the HiFive1b clocked at 320Mhz and the older HiFive1 at 200Mhz. At 48Mz for the core clock, the SPI clk will 48/8 = 6MHz with the default sckdiv setting.

[aabadie]

@kenrabold, I update the PLL settings so now the core clock runs at 192MHz. It will be difficult to do better with the HFXOSC. In master, I noticed that the CPU is running at ~300MHz but it uses HFROSC, with PLL, which is what this PR is not doing in fact... But the main problem in master is the call to PRCI_set_hfrosctrim_for_f_cpu which is super slow (it takes more than 1s to complete at boot) and cost space on ROM.
I would prefer to use the HFXOSC: it's more accurate, the CPU clock can be determined with a macro and thus it requires less ROM. Would that be ok for you if this is the default ?

[aabadie]

I pushed some commits that makes it possible to configure the boards so that they can operate at 300MHz using HFROSC with PLL: only in this case the SPI flash needs a special handling. I need to do more testing (especially with #12946, #12957 and #12953).

In the current state of this PR, all 4 clocks modes are usable: HFXOSC, HFXOSC with PLL, HFROSC and HFROSC with PLL. With the latter, one can reach ~320MHz clock but it takes more ROM space and it adds a 2s delay at boot.

[aabadie]

Finally I was able to reach a precise 320MHz core clock with HFXOSC. So now the default is 320MHz :)

[kenrabold]

Looks good. I haven't been able to try your changes because it looks like Windows based builds are broken because of this change #12773 which spits out a generates\autoconf.h file that has this garbage in it:

C:/MinGW/msys/1.0/* RIOT Configuration File */
#define CONFIG_MODULE_AUTO_INIT 1
#define CONFIG_MODULE_AUTO_INIT_SAUL 1
#define CONFIG_MODULE_BOARD 1
#define CONFIG_MODULE_CORE 1
#define CONFIG_MODULE_CORE_MSG 1
#define CONFIG_MODULE_CPU 1
#define CONFIG_MODULE_FMT 1

And causes the build to fail.

If I can figure out the issue, I'll try your updates.

[aabadie]

If I can figure out the issue, I'll try your updates.

Thanks, that would be great. I don't have a hifive1 so I'm not sure it will work at 320MHz.

[fjmolinas]

Looks good. I haven't been able to try your changes because it looks like Windows based builds are broken because of this change #12773 which spits out a generates\autoconf.h file that has this garbage in it:

C:/MinGW/msys/1.0/* RIOT Configuration File */
#define CONFIG_MODULE_AUTO_INIT 1
#define CONFIG_MODULE_AUTO_INIT_SAUL 1
#define CONFIG_MODULE_BOARD 1
#define CONFIG_MODULE_CORE 1
#define CONFIG_MODULE_CORE_MSG 1
#define CONFIG_MODULE_CPU 1
#define CONFIG_MODULE_FMT 1

And causes the build to fail.

If I can figure out the issue, I'll try your updates.

@leandrolanzieri do you have any idea of what can be happening in windows, did you try it at some point?

[aabadie]

Maybe I should squash @fjmolinas ? It will simplify the review I think.

[fjmolinas]

There is something I do not understand, @aabadie tested with BUILD_IN_DOCKER as well as I did, but the firmware produced by him works, and mine gets stuck after the esp32 boot messages. Can someone verify they do not have the issue and that it is something wrong with my config?

BUILD_IN_DOCKER=1 BOARD=hifive1b make -C tests/bench_msg_pingpong/ flash term

maybe @kaspar030?

[fjmolinas]

@aabadie re-ran tests:

Failures during test:
- [tests/cpp11_condition_variable](tests/cpp11_condition_variable/test.failed)
- [tests/gnrc_ipv6_fwd_w_sub](tests/gnrc_ipv6_fwd_w_sub/test.failed)
- [tests/pipe](tests/pipe/test.failed)
- [tests/pkg_utensor](tests/pkg_utensor/test.failed)
- [tests/ps_schedstatistics](tests/ps_schedstatistics/test.failed)
- [tests/pthread_barrier](tests/pthread_barrier/test.failed)
- [tests/shell](tests/shell/test.failed)
- [tests/xtimer_periodic_wakeup](tests/xtimer_periodic_wakeup/test.failed)
- [tests/xtimer_usleep](tests/xtimer_usleep/test.failed)

Let me know if you want the whole results, I can push to a repo.

[fjmolinas]

@aabadie I would go with this PR as is, hifive1 doesn't seem easy to buy from what I have seen, and you kept the start up code If I understand right? So lets figure out the falining tests and move on with this PR.

[aabadie]

Let me know if you want the whole results, I can push to a repo.

That would help. Locally, I don't have problems with cpp tests.

[fjmolinas]

That would help. Locally, I don't have problems with cpp tests.

here: https://github.com/fjmolinas/ci-results/tree/master/results/hifive1b

[aabadie]

I've looked into the test failures:

• tests/cpp11_condition_variable: the failed assertion is triggered because now the board uses a core clock of 320MHz. If I decrease the core clock to 200MHz, then it works. After looking into it, this is the assertion that fails:
  

  RIOT/core/priority_queue.c

  Line 60 in c310bfb

  assert(node->next != new_obj);

  
  No idea how this should be properly fixed but if I comment out the assert, the test works. This looks like a race condition, maybe in the cpp_condition_variable code.
• tests/gnrc_ipv6_fwd_w_sub, tests/ps_schedstatistics, tests/utensor, tests/pthread_barrier , tests/xtimer_periodic_wakeup: these tests are already failing in master.
• tests/pipe, tests/shell: I can't reproduce, it works on my machine ;) For tests/shell, note that it's also broken on master but it seems that this PR is fixing it (at least for me).

[aabadie]

@fjmolinas, are you ok to squash this one ?
I can open an issue to track the problems (and their fix) with the tests.

[fjmolinas]

tests/pipe, tests/shell: I can't reproduce, it works on my machine ;) For tests/shell, note that it's also broken on master but it seems that this PR is fixing it (at least for me).

Ran both tests again without issues.

[fjmolinas]

@fjmolinas, are you ok to squash this one ?
I can open an issue to track the problems (and their fix) with the tests.

Yes, please to both :)

tests/cpp11_condition_variable: the failed assertion is triggered because now the board uses a core clock of 320MHz. If I decrease the core clock to 200MHz, then it works. After looking into it, this is the assertion that fails:

I would keep the new clock speed and raise the issue, this would be our board with the fastest clock speed, it might be an issue that doesn't show at lower speeds.

[fjmolinas]

I would keep the new clock speed and raise the issue, this would be our board with the fastest clock speed, it might be an issue that doesn't show at lower speeds.

Interestingly when running multiple times it fails, suceeds on first run.

[aabadie]

@fjmolinas, rebased and squashed. I also opened #13086 for tracking the known issues.

[fjmolinas]

ACK, I've tested with hifive1b but not with hifive1, but since this board doesn't seem available for purchase (at least broadly), I think it is OK.