-
Notifications
You must be signed in to change notification settings - Fork 380
P-NUCLEO-WB55 #2017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
P-NUCLEO-WB55 #2017
Conversation
| * Internal API for stm32wbxx mcu specific code. | ||
| */ | ||
| int hal_gpio_init_af(int pin, uint8_t af_type, enum hal_gpio_pull pull, uint8_t | ||
| od); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very unusual line break
| I2C_TypeDef *hic_i2c; | ||
| volatile uint32_t *hic_rcc_reg; /* RCC register to modify */ | ||
| uint32_t hic_rcc_dev; /* RCC device ID */ | ||
| uint8_t hic_pin_sda; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for UART pins were int8_t
hw/mcu/stm/stm32wbxx/src/hal_flash.c
Outdated
| #include <mcu/stm32_hal.h> | ||
| #include "hal/hal_flash_int.h" | ||
|
|
||
| #define _FLASH_SIZE (MYNEWT_VAL(STM32_FLASH_SIZE_KB) * 1024) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_FLASH_SIZE defined locally but not use
| if (regs == TIM8) { | ||
| stm32_tmr_reg_irq(TIM8_CC_IRQn, func); | ||
| #if MYNEWT_VAL(MCU_STM32F3) || MYNEWT_VAL(MCU_STM32L4) | ||
| #if MYNEWT_VAL(MCU_STM32F3) || MYNEWT_VAL(MCU_STM32L4) || MYNEWT_VAL(MCU_STM32WB) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far STM32WB does not have TIM8 and TIM10
hw/bsp/p-nucleo-wb55/src/hal_bsp.c
Outdated
| #endif | ||
|
|
||
| #if MYNEWT_VAL(TIMER_1) | ||
| hal_timer_init(1, TIM3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIM3 is not present on stm32wb55 TIM1, TIM16, TIM17 could be considered
hw/bsp/p-nucleo-wb55/src/hal_bsp.c
Outdated
| #endif | ||
|
|
||
| #if MYNEWT_VAL(TIMER_2) | ||
| hal_timer_init(2, TIM4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no TIM4
| (void)rc; | ||
|
|
||
| #if MYNEWT_VAL(UART_0) | ||
| rc = os_dev_create((struct os_dev *) &hal_uart0, "uart0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and next line have cast space inconsistency
56c84c0 to
72597e9
Compare
|
@kasjer Could you take a look at this again, I think most review issues were tackled, apart from the |
| * (when HSE is used as system clock source, directly or through the PLL). | ||
| */ | ||
| #if !defined (HSE_VALUE) | ||
| #define HSE_VALUE 8000000U /*!< Value of the External oscillator in Hz */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure it's not 32MHz?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, fixed it (and some related changes with HSE configuration that were broken). My bad, I hadn't really tested this clock source before for this model but it's functional now.
| #define STM32_HAL_WATCHDOG_CUSTOM_INIT(x) \ | ||
| do { \ | ||
| (x)->Init.Window = IWDG_WINDOW_DISABLE; \ | ||
| } while (0) | ||
|
|
||
| /* hal_system_start */ | ||
| #define STM32_HAL_FLASH_REMAP() \ | ||
| do { \ | ||
| SYSCFG->MEMRMP = 0; \ | ||
| __DSB(); \ | ||
| } while (0) | ||
|
|
||
| /* hal_spi */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interleaving includes with definitions is quite unusual in mynewt
hw/mcu/stm/stm32wbxx/syscfg.yml
Outdated
| value: 0xff | ||
|
|
||
| STM32_FLASH_IS_LINEAR: | ||
| description: This MCU's Flash has one single sector size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sentence is strange.
maybe: all sectors have same size
hw/mcu/stm/stm32wbxx/syscfg.yml
Outdated
| - "!SPI_0_MASTER" | ||
|
|
||
| TRNG: | ||
| description: 'True Random Number Generator (RNG)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe enable word here? like in CRYPTO case
hw/mcu/stm/stm32wbxx/syscfg.yml
Outdated
| value: 1 | ||
|
|
||
| STM32_CLOCK_VOLTAGESCALING_CONFIG: | ||
| description: Voltage scale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not guess what it could be used for. But then I did not studied datasheet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One can lower the clock when running with low speed clocks, or increase it when maximum performance is desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, lower the voltage
| #ifdef TIM1 | ||
| case (uintptr_t)TIM1: | ||
| #endif | ||
| #ifdef TIM8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are TIM3, 4, 8, 9, 10, 11, 15 here for future chips?
| #endif | ||
|
|
||
| #if !IS_RCC_HCLKx(MYNEWT_VAL(STM32_CLOCK_AHBCLK2_DIVIDER)) | ||
| #error "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not much information in #error here
| * MSI Oscillator | ||
| */ | ||
| #if MYNEWT_VAL(STM32_CLOCK_MSI) | ||
| # if (MYNEWT_VAL(STM32_CLOCK_MSI_CALIBRATION) > 255) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this kind of indentation is not used in similar cases few lines down
|
|
||
| osc_init.OscillatorType |= RCC_OSCILLATORTYPE_LSI2; | ||
| osc_init.LSI2CalibrationValue = MYNEWT_VAL(STM32_CLOCK_LSI2_CALIBRATION); | ||
| #endif /* MYNEWT_VAL(STM32_CLOCK_LSI2) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid indentation
|
@kasjer All issues addressed. |
Updates the STM32 crypto driver to use the new API introduced when support for the AES module was added (an AES-only simplified cryptography driver). This currently works with F4x and WBxx (other families would required a SDK update). Signed-off-by: Fabio Utzig <utzig@apache.org>
kasjer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. I hope it works too.
Slinky works, SPI/I2C/TIM under test...