stm32 LL driver: F1/F4 (v1 controller), F3/F7/LX (v2 controller) #524
Conversation
| @@ -114,6 +114,8 @@ static const stm32_pin_func_t pin_pa3_funcs[] = { | |||
|
|
|||
| static const stm32_pin_func_t pin_pa8_funcs[] = { | |||
| PINMUX_UART(PA8, UART7, RX) | |||
| [STM32F4_PINMUX_FUNC_PA8_I2C3_SCL - 1] = | |||
There was a problem hiding this comment.
Can we not define a macro similar to PINMUX_UART()?
| @@ -71,7 +71,6 @@ | |||
| #define HAL_SDRAM_MODULE_ENABLED | |||
| #define HAL_HASH_MODULE_ENABLED | |||
| #define HAL_GPIO_MODULE_ENABLED | |||
| #define HAL_I2C_MODULE_ENABLED | |||
There was a problem hiding this comment.
This this actually accomplish anything? We don't do it for any other module. Looks like it only impacts if .c file is used, which we don't seem to build anyways.
Would prefer we don't touch ext/hal/.. files at all if possible.
There was a problem hiding this comment.
No other STM32 platform enables the i2c LL and the HAL. When it happens, the multiple definitions of I2C_SPEED_STANDARD and I2X_SPEED_FAST occur.
There was a problem hiding this comment.
Is HAL really used here? Isn't it a leftover of an early development phase?
Anyway, I agree with @galak here. We should avoid impacting /ext/hal/ code by all means.
drivers/i2c/Kconfig
Outdated
| config I2C_STM32 | ||
| bool "STM32 MCU I2C Driver" | ||
| depends on SOC_FAMILY_STM32 | ||
| depends on I2C_STM32_V1 || I2C_STM32_V2 |
drivers/i2c/Kconfig
Outdated
| bool "STM32Lx MCU I2C Interrupt Support" | ||
| depends on I2C_STM32LX | ||
| config I2C_STM32_V1 | ||
| bool "STM32 V1 Driver" |
There was a problem hiding this comment.
V1 Driver - isn't really vary useful description.
There was a problem hiding this comment.
stm32 uses two different i2c controllers but have no names.
how do you propose I describe it? gen1 and gen2, v1 and v2, controller1 and controller2?
guidelines welcome please.
There was a problem hiding this comment.
The I2C CPAL (Communication Peripheral Application Library) from ST follows the same naming. The CPAL v1 is a library which provides API for I2C peripherals of STM32F4 and CPAL v2 provides API for I2C peripherals of STM32F3 devices.
There was a problem hiding this comment.
the symbol name is fine, in the text we can do"
bool "STM32 V1 Driver - Used on F1 & F4X SoCs"
...
drivers/i2c/Kconfig
Outdated
| Enable I2C support on the STM32 F1 and F4X family of processors | ||
|
|
||
| config I2C_STM32_V2 | ||
| bool "STM32 V2 Driver" |
There was a problem hiding this comment.
V2 Driver - isn't really vary useful description.
drivers/i2c/Makefile
Outdated
| @@ -7,5 +7,7 @@ obj-$(CONFIG_I2C_NRF5) += i2c_nrf5.o | |||
| obj-$(CONFIG_I2C_QMSI) += i2c_qmsi.o | |||
| obj-$(CONFIG_I2C_QMSI_SS) += i2c_qmsi_ss.o | |||
| obj-$(CONFIG_I2C_SBCON) += i2c_sbcon.o | |||
| obj-$(CONFIG_I2C_STM32LX) += i2c_stm32lx.o | |||
| obj-$(CONFIG_I2C_STM32_V2) += i2c_ll_stm32_v2.o i2c_ll_stm32.o | |||
| obj-$(CONFIG_I2C_STM32_V1) += i2c_ll_stm32_v1.o i2c_ll_stm32.o | |||
There was a problem hiding this comment.
missed that one. will do in the next patchset once the driver is reviewed
drivers/i2c/Kconfig
Outdated
| config I2C_STM32LX | ||
| bool "STM32Lx MCU I2C Driver" | ||
| depends on SOC_FAMILY_STM32 && SOC_SERIES_STM32L4X | ||
| config I2C_STM32 |
There was a problem hiding this comment.
Is there are 2 different drivers, remove I2C_STM32, doesn't add any particular value.
|
I validated also the v1 i2c controller on STM32F1 family. |
drivers/i2c/Kconfig
Outdated
| Enable I2C support on the STM32 F1, L1 and F4X family of processors | ||
|
|
||
| config I2C_STM32_V2 | ||
| bool "STM32 V2 Driver (F3/F7/L4X)" | ||
| depends on SOC_FAMILY_STM32 && SOC_SERIES_STM32L4X |
There was a problem hiding this comment.
is the depends correct if we can use this on F3 & F7 as well?
There was a problem hiding this comment.
wouldn't it be better if we change the depends when we add the i2c support (pinmux driver changes, soc pinmux changes etc) on the rest of the families?
There was a problem hiding this comment.
That the help & 'bool' should only reference what is supported.
drivers/i2c/Kconfig
Outdated
| bool "STM32Lx MCU I2C Driver" | ||
| config I2C_STM32_V1 | ||
| bool "STM32 V1 Driver (F1/L1/F4X)" | ||
| depends on SOC_FAMILY_STM32 && SOC_SERIES_STM32F4X |
There was a problem hiding this comment.
Is the depends correct for L1 & F1?
There was a problem hiding this comment.
wouldn't it be better if we change the depends when we add the i2c support (pinmux driver changes, soc pinmux changes etc) on the rest of the families?
There was a problem hiding this comment.
That the help & 'bool' should only reference what is supported.
drivers/i2c/Makefile
Outdated
| obj-$(CONFIG_TWIHS_SAM) += twihs_sam.o | ||
|
|
drivers/i2c/i2c_ll_stm32_v1.c
Outdated
| * | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * I2C Driver for: STM32F1 and STM32F4 |
drivers/i2c/i2c_ll_stm32.h
Outdated
| do { p++; len--; } while (0) | ||
|
|
||
| s32_t msg_write(struct device *dev, struct i2c_msg *msg, u32_t flags, | ||
| u16_t saddr); |
There was a problem hiding this comment.
fix line wrap to lineup with struct on previous line.
There was a problem hiding this comment.
will you fix checkpatch.pl to catch this?
drivers/i2c/i2c_ll_stm32.h
Outdated
| s32_t msg_write(struct device *dev, struct i2c_msg *msg, u32_t flags, | ||
| u16_t saddr); | ||
| s32_t msg_read(struct device *dev, struct i2c_msg *msg, u32_t flags, | ||
| u16_t saddr); |
There was a problem hiding this comment.
see above line wrap comment
drivers/i2c/i2c_ll_stm32.h
Outdated
| s32_t msg_read(struct device *dev, struct i2c_msg *msg, u32_t flags, | ||
| u16_t saddr); | ||
| s32_t i2c_configure_timing(struct device *dev, u32_t clock); | ||
| void i2c_stm32_ev_isr(void *args); |
There was a problem hiding this comment.
can we rename i2c_stm32_ev_isr to i2c_stm32_event_isr()
and i2c_stm32_er_isr -> i2c_stm32_error_isr()
There was a problem hiding this comment.
um...well if you insist ok
drivers/i2c/i2c_ll_stm32.c
Outdated
|
|
||
| #define OPERATION(msg) (((struct i2c_msg *) msg)->flags & I2C_MSG_RW_MASK) | ||
|
|
||
| static int i2c_stm32_transfer(struct device *dev, struct i2c_msg *message, |
There was a problem hiding this comment.
message -> msg
messages -> num_msgs
There was a problem hiding this comment.
Agreed this would help avoiding confusion at first read
drivers/i2c/i2c_ll_stm32.c
Outdated
| LL_I2C_Enable(i2c); | ||
|
|
||
| current = message; | ||
| while (messages > 0) { |
There was a problem hiding this comment.
I think I for loop is easier to understand:
for (int i = 0; i < num_msgs; i++, msg++)
{
...
}
There was a problem hiding this comment.
I dont agree. please let me know if is a must or a recommended
There was a problem hiding this comment.
the while loop is clear and easy to understand, I don't see why we need to change it with a for loop.
drivers/i2c/i2c_ll_stm32.c
Outdated
| if (current->len > 255) | ||
| return -EINVAL; | ||
|
|
||
| if (messages > 1) { |
There was a problem hiding this comment.
probably should have a comment here about what is going on.
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
We define a variable to pickup a default for the bossa binary, however we weren't using it. Lets do so now. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
If we have more than one board of a given type we need to be able to specify the board_id to select which specific board that the pyocd command should target. Introduce an environment variable PYOCD_BOARD_ID to we can set that will get passed to the pyocd command that needs it. Here's an example: $ make -C samples/hello_world/ BOARD=frdm_k64f flash PYOCD_BOARD_ID=1234 Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Multiple Kinetis SoCs have the same pinmux hardware as the k64 and can use the same mcux driver, so rename the dts to be more generic. Signed-off-by: Maureen Helm <maureen.helm@nxp.com>
Multiple Kinetis SoCs have the same gpio hardware as the k64 and can use the same mcux driver, so rename the dts to be more generic. Also fixes some stranded references to kw41z-gpio to the new kinetis-gpio. Signed-off-by: Maureen Helm <maureen.helm@nxp.com>
Renames k64sim to nxp,k64f-sim to be more consistent with other files. The sim hardware can vary across Kinetis SoCs, so this dts is not made to be generic. Signed-off-by: Maureen Helm <maureen.helm@nxp.com>
|
if you can rebase on latest branch that would be great. |
| @@ -22,6 +22,7 @@ | |||
| #define STM32F4_PINMUX_FUNC_PA3_USART2_RX STM32_PINMUX_FUNC_ALT_7 | |||
|
|
|||
| #define STM32F4_PINMUX_FUNC_PA8_UART7_RX STM32_PINMUX_FUNC_ALT_8 | |||
| #define STM32F4_PINMUX_FUNC_PA8_I2C3_SCL STM32_PINMUX_FUNC_ALT_4 | |||
There was a problem hiding this comment.
For clarity, store #defined depending on ALT number
|
|
||
| #define STM32F4_PINMUX_FUNC_PB5_UART5_RX STM32_PINMUX_FUNC_ALT_11 | ||
|
|
||
| #define STM32F4_PINMUX_FUNC_PB6_USART1_TX STM32_PINMUX_FUNC_ALT_7 | ||
| #define STM32F4_PINMUX_FUNC_PB6_UART5_TX STM32_PINMUX_FUNC_ALT_11 | ||
| #define STM32F4_PINMUX_FUNC_PB6_I2C1_SCL STM32_PINMUX_FUNC_ALT_4 |
|
|
||
| #define STM32F4_PINMUX_FUNC_PB7_USART1_RX STM32_PINMUX_FUNC_ALT_7 | ||
| #define STM32F4_PINMUX_FUNC_PB7_I2C1_SDA STM32_PINMUX_FUNC_ALT_4 |
|
|
||
| #define STM32F4_PINMUX_FUNC_PB8_UART5_RX STM32_PINMUX_FUNC_ALT_11 | ||
| #define STM32F4_PINMUX_FUNC_PB8_I2C1_SCL STM32_PINMUX_FUNC_ALT_4 |
|
|
||
| #define STM32F4_PINMUX_FUNC_PB9_UART5_TX STM32_PINMUX_FUNC_ALT_11 | ||
| #define STM32F4_PINMUX_FUNC_PB9_I2C1_SDA STM32_PINMUX_FUNC_ALT_4 |
| @@ -19,6 +19,11 @@ static const struct pin_config pinconf[] = { | |||
| {STM32_PIN_PA9, STM32F3_PINMUX_FUNC_PA9_USART1_TX}, | |||
| {STM32_PIN_PA10, STM32F3_PINMUX_FUNC_PA10_USART1_RX}, | |||
| #endif /* CONFIG_UART_STM32_PORT_1 */ | |||
| #ifdef CONFIG_I2C_1 | |||
| /* I2C2 is used for NFC, STSAFE, ToF & MEMS sensors */ | |||
There was a problem hiding this comment.
Please remove comment (copy/paste issue)
| @@ -14,5 +14,6 @@ config SOC_SERIES_STM32F1X | |||
| select HAS_STM32CUBE | |||
| select CPU_HAS_SYSTICK | |||
| select CLOCK_CONTROL_STM32_CUBE if CLOCK_CONTROL | |||
| select I2C_STM32_V1 if I2C | |||
There was a problem hiding this comment.
Based on this, remove CONFIG_I2C_STM32_V1 from board _defconfig
| @@ -15,5 +15,6 @@ config SOC_SERIES_STM32F3X | |||
| select CPU_HAS_SYSTICK | |||
| select HAS_STM32CUBE | |||
| select CLOCK_CONTROL_STM32_CUBE if CLOCK_CONTROL | |||
| select I2C_STM32_V2 if I2C | |||
| @@ -16,5 +16,6 @@ config SOC_SERIES_STM32F4X | |||
| select CPU_HAS_MPU | |||
| select CPU_HAS_SYSTICK | |||
| select CLOCK_CONTROL_STM32_CUBE if CLOCK_CONTROL | |||
| select I2C_STM32_V1 if I2C | |||
| @@ -14,5 +14,6 @@ config SOC_SERIES_STM32L4X | |||
| select SYS_POWER_LOW_POWER_STATE_SUPPORTED | |||
| select CPU_HAS_SYSTICK | |||
| select CLOCK_CONTROL_STM32_CUBE if CLOCK_CONTROL | |||
| select I2C_STM32_V2 if I2C | |||
Add I2C1 pins on PB6, PB7, PB8, PB9 Add I2C2 pins on PB3, PB10, PB11, PF0, PF1, PH4, PH5 Add I2C3 pins on PA8, PB4, PC9, PH7, PH8 Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com> Reviewed-by: Jorge Ramirez-Oritz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Enablign the HAL and the LL flags duplicate definitions when compiling in driver. Since the i2c HAL is not needed, disable it. This is a HAL issue that needs to be addressed (not zephyr) Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Supports STM32 F1/F4 (v1 controller) and STM32 F3/L4X (v2 controller) v1 could also support L1X. v2 could also support F7X. Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com> Reviewed-by: Yannis Damigos <giannis.damigos@gmail.com> Tested-by: Yannis Damigos <giannis.damigos@gmail.com> Tested-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Replace native driver with the LL driver Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Add I2C support for Carbon from 96Boards. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com> Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Build stm32f1xx_ll_i2c when you enable I2C support for the stm32f1 family. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>
Enable support for STM32F1 family. Tested on Olimexino-STM32 board. Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>
Add I2C1 pins on PB6, PB7, PB8, PB9 Add I2C2 pins on PB10, PB11 Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>
Add I2C support for Olimexino-STM32 Signed-off-by: Yannis Damigos <giannis.damigos@gmail.com>
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
This commit also updates the board dependencies associated to this change. Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
ldts
force-pushed
the
st/i2c_ll
branch
2 times, most recently
from
00220be
to
8fec5ea
Compare
| Additional device support is left for the user to implement. | ||
|
|
||
| More information on hooking up peripherals and lengthy how to articles can be | ||
| found at `EmbedJoural`_. |
There was a problem hiding this comment.
Merged PR #565 (on the arm branch) fixes issues with this .rst file (EmbedJoural is misspelled, lots of missing +'s on tables below...)
Fixes issue zephyrproject-rtos#524. Signed-off-by: Geoff Gustafson <geoff@linux.intel.com>
…_token Increase coverage
This patchset replaces the current stm32lx i2c driver with a generic implementation covering all the STM32 controllers.
@ydamigos implemented support for the v1 controller.
@erwango defined the driver requirements and initial tests.
F4X (v1) has been tested on Nucleo F401re using the Grove LCD RGB sample code in Zephyr in polling/IRQ mode.
The F3 and LX (v2) family were also validated.