Support for STM32L0 MCUs and Dragino LGT92 device #1561

ofauchon · 2021-01-05T14:35:24Z

This PR provides support for :

STM32L0 microcontroller series.
Dragino LGT92 device (which is based on stm32l072czt6 )

Working:

LPUART and UART
GPIOs
SPI
Timers

Not working yet:

i2c
power saving (LP modes)

- lgt92.json now inherits stm32l0x2.json

aykevl

Thank you, looks good to me! I have a number of code cleanliness issues but nothing major.

src/machine/board_lgt92.go

aykevl · 2021-01-05T22:27:39Z

src/machine/board_lgt92.go

+	stm32.RCC.IOPENR.SetBits(stm32.RCC_IOPENR_IOPAEN)
+	stm32.RCC.IOPENR.SetBits(stm32.RCC_IOPENR_IOPBEN)
+	stm32.RCC.IOPENR.SetBits(stm32.RCC_IOPENR_IOPCEN)
+
+	// Enable NVM (eeprom)
+	stm32.RCC.AHBENR.SetBits(stm32.RCC_AHBENR_MIFEN)
+
+	// Enable Leds GPIO output
+	LED_RED.Configure(PinConfig{Mode: PinOutput})
+	LED_GREEN.Configure(PinConfig{Mode: PinOutput})
+	LED_BLUE.Configure(PinConfig{Mode: PinOutput})
+
+	LED_RED.Low()
+	LED_GREEN.Low()
+	LED_BLUE.Low()


Why are you doing this in the board file?

Configuring outputs needs to be done by the program, unless it's really necessary they should be configured by a program that wants to use these LEDs.

Is there any support for EEPROM? If not, I would suggest leaving it disabled (enabling unused peripherals may cost power).

If you need to enable the GPIO peripheral (stm32.RCC.IOPENR.SetBits(stm32.RCC_IOPENR_IOPAEN)), this should be done in Pin.Configure. It looks like this is already done, so these lines are unnecessary.

aykevl · 2021-01-05T22:29:49Z

src/machine/machine_stm32l0.go

+		//	case 5:
+		//		return stm32.GPIOF
+		//	case 6:
+		//		return stm32.GPIOG


If GPIOF etc. don't exist, you don't need to include them here. If you want to explain why you left them out, you could add a comment saying "GPIOF etc. do not exist on this chip".

aykevl · 2021-01-05T22:32:11Z

src/machine/machine_stm32l0.go

+		//	case unsafe.Pointer(stm32.TIM8): // TIM8 clock enable
+		//		stm32.RCC.APB2ENR.SetBits(stm32.RCC_APB2ENR_TIM8EN)
+		//	case unsafe.Pointer(stm32.TIM1): // TIM1 clock enable
+		//		stm32.RCC.APB2ENR.SetBits(stm32.RCC_APB)


This is again code that should be removed: TIM1 and TIM8 do not seem to exist on this chip.

aykevl · 2021-01-05T22:34:41Z

src/machine/machine_stm32l0x2.go

+
+	// These are based on APB2 clock frquency (84MHz on the discovery board)
+	// TODO: also include the MCU/APB clock setting in the equation
+	switch true {


You can leave out the true here, a switch can also work as a replacement for a long if / else if / else chain.

Suggested change

switch true {

switch {

aykevl · 2021-01-05T22:37:40Z

src/machine/machine_stm32l0x2.go

+		conf = stm32.SPI_PCLK_2
+	default:
+		// None of the specific baudrates were selected; choose the lowest speed
+		conf = stm32.SPI_PCLK_256


Ideally we'd default to 4MHz, but if that is not possible you could use a value just below that.
You can add such a check at the top by checking for 0:

if config.Frequency == 0 { config.Frequency = 4e6 }

aykevl · 2021-01-05T22:46:05Z

src/machine/machine_stm32l0x2.go

+	case localFrequency < 328125:
+		conf = stm32.SPI_PCLK_256


It would be useful to make this calculation explicit. For example:

const apb2Frequency = 84e6 // ... switch { case localFrequency >= apb2Frequency / 2: conf = stm32.SPI_PCLK_2 case localFrequency >= apb2Frequency / 4: conf = stm32.SPI_PCLK_4 // ... case localFrequency >= apb2Frequency / 128: conf = stm32.SPI_PCLK_128 default: conf = stm32.SPI_PCLK_256 }

This way the calculation is very explicit. This kind of calculation is always done at compile time, so you don't need to worry about any overhead it might introduce.
You can take a look here as an example: #1446

aykevl · 2021-01-05T22:49:50Z

targets/stm32l0x2.json

+    "flash-method": "openocd",
+    "openocd-interface": "stlink-v2",


Suggested change

"flash-method": "openocd",

"openocd-interface": "stlink-v2",

These two lines are specific to the board, so shouldn't be in the chip specification.

aykevl · 2021-01-05T22:51:20Z

targets/stm32l0x2.json

+        "--target=armv6m-none-eabi",
+        "-Qunused-arguments"
+    ],
+    "linkerscript": "targets/stm32l072czt6.ld",


Suggested change

"linkerscript": "targets/stm32l072czt6.ld",

Because this linker script is for a more specific chip (with a given amount of memory), it shouldn't be included in the more generic chip family specification.

aykevl · 2021-01-05T23:07:25Z

And before I forget: can you also add a smoke test to the Makefile? You can see a list of commands there already to run the smoke tests.

Add default LED declaration so example compiles Remove useless hardware init and comments Move all machine_stm32l0x2 code back to machine_stm32l0 as it'll probably work for stm32l0x1 Move Flash-method,linkerscript... from chip target to board target json

…elds.go file.

…ibility

ofauchon · 2021-01-07T15:45:44Z

Hi @aykevl,

Thanks for your detailed review, and all the suggestions.
I think I made all the proposed changes.

Please tell me if you have other improvements.

Olivier

aykevl

Thank you for the fixes! Looks good to me now.

deadprogram · 2021-01-08T21:26:39Z

I will squash/merge this PR now. Thank you very much for the contribution @ofauchon

ofauchon added 3 commits January 5, 2021 09:13

Add support for stm32l0 familly, and Dragino LGT92 Board

a327923

- Dead code removed

931cbcb

- lgt92.json now inherits stm32l0x2.json

removed lgt92 build tag in stm32l0x2.json

b57d0f1

aykevl reviewed Jan 5, 2021

View reviewed changes

ofauchon added 2 commits January 6, 2021 22:39

Add smoketest for lgt92

14eb657

Add default LED declaration so example compiles Remove useless hardware init and comments Move all machine_stm32l0x2 code back to machine_stm32l0 as it'll probably work for stm32l0x1 Move Flash-method,linkerscript... from chip target to board target json

Moved runtime_stm32l0x2 in runtime_stm32l0

7e7b8d1

ofauchon changed the title ~~Support for STM32L0x MCUs and Dragino LGT92 device~~ Support for STM32L0 MCUs and Dragino LGT92 device Jan 6, 2021

ofauchon added 3 commits January 7, 2021 01:10

Add some registers const to avoid the use of an extra stm32l0x2-bitfi…

7e246d7

…elds.go file.

Remove binary integer prefix 0b, for Golang<1.13 backward compatibility

d644a93

Remove more binary integer prefix 0b, for Golang<1.13 backward compat…

d83e5ed

…ibility

aykevl mentioned this pull request Jan 8, 2021

stm32: use stm32-rs SVDs which are of much higher quality #1557

Merged

aykevl approved these changes Jan 8, 2021

View reviewed changes

deadprogram merged commit 65caf77 into tinygo-org:dev Jan 8, 2021

ofauchon deleted the stm32l0 branch May 31, 2021 20:46

Support for STM32L0 MCUs and Dragino LGT92 device #1561

Support for STM32L0 MCUs and Dragino LGT92 device #1561

Uh oh!

Conversation

ofauchon commented Jan 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aykevl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aykevl Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

aykevl Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

aykevl Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

aykevl Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

aykevl Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

aykevl Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

aykevl Jan 5, 2021 • 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!

aykevl Jan 5, 2021 • 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!

aykevl commented Jan 5, 2021

Uh oh!

ofauchon commented Jan 7, 2021

Uh oh!

aykevl left a comment

Choose a reason for hiding this comment

Uh oh!

deadprogram commented Jan 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ofauchon commented Jan 5, 2021 •

edited

Loading

aykevl Jan 5, 2021 •

edited

Loading

aykevl Jan 5, 2021 •

edited

Loading