Skip to content

Conversation

@yannishuber
Copy link
Contributor

@yannishuber yannishuber commented Jun 1, 2020

This PR adds support for the Kendryte K210 chip and the Sipeed MAix Bit board.

TODO:

  • Runtime code
  • UART
  • GPIO
  • GPIO interrupts
  • SPI
  • I²C
  • Unit tests

@yannishuber
Copy link
Contributor Author

The CMSIS-SVD repository has temporarly been changed and will be reset to the original upstream once cmsis-svd/cmsis-svd#104 is merged.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good. I have left a number of comments inline, however.
Regarding cmsis-svd, while I would prefer to use upstream, I think practically it would be better to fork it into tinygo-org (as we did previously) and keeping it there, even as cmsis-svd upstream merges PRs. Switching between submodule remotes has resulted in trouble before so let's do it once for good.

Looking at this PR, there are really three somewhat distinct changes (two necessary as precondition to K210 support):

  • Adding support for non-default code models.
  • Adding support for 64-bit RISC-V.
  • Adding support for the K210 chip.

You could cleanly separate these in different commits (with a rebase and force-push) or actually make separate PRs, what you prefer. Separate PRs may be faster to merge individually.

@yannishuber
Copy link
Contributor Author

@aykevl I made two PRs in the tinygo-org/cmsis-svd repo to update it and to incorporate the unmerged fix (tinygo-org/cmsis-svd#3 and tinygo-org/cmsis-svd#4). I will update the submodule repository as soon as these are merged.

I will also separate this pull request into 3 distinct ones as suggested.

@deadprogram
Copy link
Member

All of the PRs to the tinygo fork of cmsis-svd are done. Please continue with the rest of the steps so we can get this merged! Thank you.

@deadprogram
Copy link
Member

I should be receiving a Sipeed MAix BiT board next week, so I can also help test.

@deadprogram
Copy link
Member

So this PR is the precursor to adding the board and machine files for the MAiX BiT, correct? @yannishuber are you planning on doing this? I just got my boards today. 😸

@yannishuber
Copy link
Contributor Author

@deadprogram yes exactly, i also have this board and I am currently working on it. It should be done soon.

@deadprogram
Copy link
Member

So should this PR still be considered draft, or should we look at merging it?

@deadprogram
Copy link
Member

@yannishuber is this still a draft PR, or should it be able to be merged into dev?

@yannishuber
Copy link
Contributor Author

@deadprogram Sorry I didn't see your first reply. No I was thinking of adding the code related to the maix-bit in this PR but I can also make a separate PR if you want. I will work on this today.

@deadprogram
Copy link
Member

It can be in the same PR, since that lets us test the full implementation that way. Thank you!

@yannishuber yannishuber changed the title Add new kendryte k210 chip riscv: add new kendryte k210 chip Jun 16, 2020
Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This chip seems to be pretty similar to the SiFive fe310, so much that I wonder whether some code can be reused between them. That would require putting the SiFive and Kendryte vendor code in the same package (perhaps device/riscv?).

@yannishuber yannishuber force-pushed the maix-bit branch 2 times, most recently from 1e193f5 to 06b4ed1 Compare June 23, 2020 11:00
@niaow niaow added this to the v0.15 milestone Jun 27, 2020
@yannishuber yannishuber force-pushed the maix-bit branch 3 times, most recently from 88694a9 to 82c9676 Compare June 30, 2020 12:16
@yannishuber
Copy link
Contributor Author

...now that each goroutine has its own stack.

@aykevl is that also the case with the coroutines scheduler?

In any case, this can easily be added at a later date.

Well in that case I think this PR is finally ready.

@yannishuber yannishuber marked this pull request as ready for review July 4, 2020 13:35
@aykevl
Copy link
Member

aykevl commented Jul 4, 2020

You may want to rebase on the dev branch to make sure this PR still works after #1142 has been merged.

@yannishuber
Copy link
Contributor Author

You may want to rebase on the dev branch to make sure this PR still works after #1142 has been merged.

I already did that. The changes from #1142 are incorporated.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready to merge. I'm a bit unsure about the nonstandard API for pin modes, but I guess it's fine for now as it's still usable this way (but with fixed pins).

@deadprogram
Copy link
Member

OK, the time has come. Now rebasing into dev

Incredble work @yannishuber and fantastic effort all around. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants