STM32F103XX UART implementation #107

deadprogram · 2018-12-14T12:42:18Z

This PR implements UART0 for the STM32F103C8T6 aka Bluepill board.

You must have an FTDI adaptor/cable to use the UART on the Bluepill board. You can either use the default TX/RX pins PA9/PA10 or else use the alternate pins PB6 and PB7 by specifying the UART configuration like this:

machine.UART0.Configure(machine.UARTConfig{TXPin: machine.PB6, RXPin: machine.PB7})

Signed-off-by: Ron Evans <ron@hybridgroup.com>

aykevl · 2018-12-14T17:03:05Z

src/machine/machine_stm32.go


 type GPIOMode uint8

+const SystemClockSpeed = 72000000


board_arduino.go uses CPU_FREQUENCY.
I think this is a better name for the constant, but it would be best to unify them. That can be done in a different PR, however.

This is actually the system clock frequency, not the CPU frequency. That value could in theory change based on the user's clock register settings. 72Mhz is such as typical number used that it needed go be made into some kind of const, right? Maybe we need to have a more global group of consts with different common speed values.

What's exactly the difference between system clock and CPU frequency? Aren't they both just the CPU frequency?

The system clock at ARM can be based on several different sources, including the CPU freq. and can be changed at runtime.

The CPU frequency for AVR is set by fuses that can only be changed when flashing the chip and does not have another source for the system clock as far as I know.

If any change is called for to unify terms it might be to have AVR use the term SystemClockSpeed but probably not worth it, since typical AVR docs refer to this as CPU frequency.

I've been looking into SystemClockSpeed a bit but it appears to be specific to CMSIS, and it doesn't seem to be used much. Looks like it is intended to easily communicate the current clock frequency across the system.
Cortex-M devices appear to be more flexible in their clock configurations, and can sometimes change the CPU frequency at runtime (like in the case of stm32, but not in the case of nrf). I think changing the frequency makes sense in some specific circumstances (like variable power sources), but in general I can't see a reason why you wouldn't want to run at the max clock speed at all times. By using a high CPU frequency, computations are faster to complete and the chip can go to sleep faster which saves energy.
Please correct me if I'm wrong, but in this light, the SystemClockSpeed global variable as used in CMSIS doesn't seem to make a lot of sense in the context of TinyGo. The only thing that is relevant is the (const) CPU frequency used on any particular configuration, which may depend on the board (whether there is a crystal available, for example). And thus it would be equivalent to CPU_FREQUENCY on the AVR.

OK, changed.

aykevl · 2018-12-15T10:53:11Z

src/machine/machine_stm32.go

 func (p GPIO) getPort() *stm32.GPIO_Type {
+	// Also enable clock for that GPIO port.
+	// Do this always, as it isn't known whether the clock has already been
+	// enabled.


getPort is also used in Set, so the peripheral will be enabled every time a pin is toggled, which seems wasteful to me.
It's perhaps a little bit of duplication, but can you maybe move the switch to enable the peripheral into the Configure function?

That seems like a good suggestion. I will do that.

aykevl · 2018-12-15T11:11:20Z

src/machine/machine_stm32.go

+	// USART1 is the first hardware serial port on the STM32.
+	// Both UART0 and UART1 refers to USART1.
+	UART0 = &UART{}
+	UART1 = UART0


So there is no UART0 on the STM32?
It seems a bit wrong this way but I guess exposing UART0 for USART1 is the best thing to do here, then.

Somewhat offtopic: I think it would be nice to not have UART0, UART1 etc. but just [...]UART at some point in the future, to also make it easier to enumerate UART devices, for example in periph. That means the first UART is always UART[0]. This is even more relevant to other peripherals like I2C, especially on chips that don't have fixed pins for peripherals.

aykevl · 2018-12-15T11:19:14Z

src/machine/uart.go

 type UARTConfig struct {
 	BaudRate uint32
+	TXPin    uint8
+	RXPin    uint8


For I2C and SPI, the -Pin suffix is not used, they simply use MOSI, SDA, etc. It would be nice to keep the naming of these pins consistent.

OK I will make that change.

aykevl · 2018-12-15T11:22:21Z

src/runtime/runtime_stm32.go


+const (
+	// Flash Access Control Register flag values.
+	FLASH_ACR_LATENCY_0 = 0x00000004


This value looks suspicious. Shouldn't this be 0x00000001?

Yes, you are correct.

aykevl · 2018-12-15T11:38:21Z

src/runtime/runtime_stm32.go

+	RCC_CFGR_PLLMUL_14 = 0x00300000
+	RCC_CFGR_PLLMUL_15 = 0x00340000
+	RCC_CFGR_PLLMUL_16 = 0x00380000
+)


It doesn't seem to me like these constants belong in the runtime or machine package. It's very unfortunate that the .svd files provided by STMicro do not contain these bitfields, but I don't think putting them directly in the machine and runtime packages isn't the right thing to do either. Maybe they can be moved to src/device/stm32/stm32f103xx-bitfields.go, for example?

Those src/device directories are not currently under version control, which is why I did not put them there. Any other suggestions on how to better handle?

There is src/device/nrf, src/device/arm, and src/device/avr. I think it's fine to add another directory.
Git doesn't track directories, only files. A file that is already tracked is not ignored by .gitignore.

I could use the -f flag to force the commit of such a bitfields file to the correct directory src/device/stm32/stm32f103xx-bitfields.go which seems like would accommodate this use case.

I think I did it before simply by commenting out the lines in the .gitignore file before adding and uncommenting again afterwards. But maybe you can also try adding an exception to the .gitignore file.

…or future board imeplementation Signed-off-by: Ron Evans <ron@hybridgroup.com>

Signed-off-by: Ron Evans <ron@hybridgroup.com>

deadprogram · 2018-12-17T14:50:54Z

I think all the feedback has now been incorporated into this PR. Please let me know if anything else is needed.

Signed-off-by: Ron Evans <ron@hybridgroup.com>

…32f103xx Signed-off-by: Ron Evans <ron@hybridgroup.com>

aykevl · 2018-12-18T16:21:24Z

Merged in 47b667a.

Support reusable workflows in the same repository

deadprogram added 2 commits December 13, 2018 16:12

machine/stm32: WIP on UART implementation

8f983cf

Signed-off-by: Ron Evans <ron@hybridgroup.com>

machine/stm32: fully functional UART0 for STM32F103C8T6 aka Bluepill

67ab338

Signed-off-by: Ron Evans <ron@hybridgroup.com>

aykevl reviewed Dec 15, 2018

View reviewed changes

deadprogram added 2 commits December 17, 2018 15:42

machine/stm32: complete support for stm32f103xx UART and leave room f…

ef4fdf8

…or future board imeplementation Signed-off-by: Ron Evans <ron@hybridgroup.com>

machine/stm32: correct value for FLASH_ACR_LATENCY_0 flag on stm32f103xx

efdd48b

Signed-off-by: Ron Evans <ron@hybridgroup.com>

deadprogram changed the title ~~STM32 UART implementation~~ STM32F103XX UART implementation Dec 17, 2018

deadprogram added 2 commits December 17, 2018 21:11

machine/stm32: change SystemClockSpeed to CPU_FREQUENCY on stm32f103xx

74b5e8b

Signed-off-by: Ron Evans <ron@hybridgroup.com>

machine/stm32: cleanly use USART1 register for UART1 interface on stm…

f86083f

…32f103xx Signed-off-by: Ron Evans <ron@hybridgroup.com>

aykevl closed this Dec 18, 2018

adrianbn mentioned this pull request May 1, 2021

Goland + TinyGo + Modules #1847

Closed

ZauberNerd pushed a commit to ZauberNerd/tinygo that referenced this pull request Mar 4, 2022

Merge pull request tinygo-org#107 from jsok/workflow-call-local

62628dc

Support reusable workflows in the same repository

STM32F103XX UART implementation #107

STM32F103XX UART implementation #107

Uh oh!

Conversation

deadprogram commented Dec 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aykevl Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aykevl Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deadprogram commented Dec 17, 2018

Uh oh!

aykevl commented Dec 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aykevl Dec 17, 2018 •

edited

Loading

aykevl Dec 17, 2018 •

edited

Loading