Skip to content

boards: nucleo_f103rb: enables I2C_1 and I2C_2 #12246

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

Closed
wants to merge 5 commits into from
Closed

boards: nucleo_f103rb: enables I2C_1 and I2C_2 #12246

wants to merge 5 commits into from

Conversation

mattzzw
Copy link

@mattzzw mattzzw commented Dec 30, 2018

Resolves #12243

Signed-off-by: Matthias Wientapper [email protected]

Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

Now that you enable I2C on the board, could you configure boards arduino connector (uart, spi and i2c)? Check nucleo_f746zg for reference.

@mattzzw
Copy link
Author

mattzzw commented Dec 30, 2018

@ydamigos, first I'd like to see I2C working on the nucleo board before I look at other board's configurations.

Right now, a simple i2c_read lets the system hangs in stm32_i2c_msg_read at

_STATIC_INLINE uint32_t LL_I2C_IsActiveFlag_SB(I2C_TypeDef *I2Cx)
{
  return (READ_BIT(I2Cx->SR1, I2C_SR1_SB) == (I2C_SR1_SB));
}

(Just learning how to debug with Eclipse...)
I2C scanner example code shows the same behavior.
But that might be related to a different matter. I will have access to a scope in couple of days again.

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Besides activating devices in dts file, you'll need to configure activation in Kconfig.defconfig (cf nucleo_f401re for instance) and configure pinmux as well.

@mattzzw
Copy link
Author

mattzzw commented Jan 2, 2019

@erwango , thanks for the help - I got I2C working finally.
I activated only I2C_1 in Kconfig.defconfig and in pinmux (as does stm32_min_dev which is best reference imho).

Right now the default assignment is SCL/PB6, SDA/PB7.
However, in order for the board's Arduino headers to match the Arduino pinout I2C_1 needs to be remapped to SCL/PB8, SDA/PB9 via SP1_REMAP bit of the AFIO_MAPR register.
Should PB8/PB9 be the default mapping? If yes, where would be the best spot to do that?

@erwango
Copy link
Member

erwango commented Jan 3, 2019

Should PB8/PB9 be the default mapping?

This would make sense.

If yes, where would be the best spot to do that?

Theoretically should go in drivers/pinmux/stm32/pinmux_stm32.c.

@mattzzw
Copy link
Author

mattzzw commented Jan 3, 2019

Hm. That would add a board dependency to drivers/pinmux/stm32/pinmux_stm32.c - Is that a good thing?

I succeeded remapping the pin to PB8/PB9 by adding
(*(volatile unsigned int *)(0x40010004)) |= 2; (AFIO_MAPR, I2C1_REMAP bit)
to main.c. I don't know how to bit bang properly zephyr style yet.

If PB8/PB9 are used in pinmux.c then I2C1_REMAP must be set.

I'm struggling to see
a) where setting 2C1_REMAPcould best be located as it is clearly a specific board thing. Also I think AFIO regs can only be written when APB2 clock is enabled (RCC_APB2ENR, AFIO_EN).
b) how to use the right header files / use the correct register manipulation functions/macros.

I know, it's only I2C but zephyr is quite new to me. :-)

@erwango
Copy link
Member

erwango commented Jan 4, 2019

Hm. That would add a board dependency to drivers/pinmux/stm32/pinmux_stm32.c - Is that a good thing?

Indeed, we should avoid to make it as a board dependency. I first though it could be done in board pinmux.c func: pinmux_stm32_init.
But this is actually a soc dependent code. All STM32F1XX based boards that want to have this pin config will need to do this. Hence proposal of pinmux driver. But this doesn't need to be a board dependency. I think (but didn't look in detail, so I could be wrong) we could actually detect from the pinconf that this remap is required and apply it, can't we?
At least if possible, I'd like this could be tried. If too much of a burden let's set it back in pinmux_stm32_init.

I succeeded remapping the pin to PB8/PB9 by adding
(*(volatile unsigned int *)(0x40010004)) |= 2; (AFIO_MAPR, I2C1_REMAP bit)
to main.c. I don't know how to bit bang properly zephyr style yet.

If PB8/PB9 are used in pinmux.c then I2C1_REMAP must be set.

I'm struggling to see
a) where setting 2C1_REMAPcould best be located as it is clearly a specific board thing. Also I think AFIO regs can only be written when APB2 clock is enabled (RCC_APB2ENR, AFIO_EN).
b) how to use the right header files / use the correct register manipulation functions/macros.

There are dedicated helper function from STM32Cube SDK that might be handy here, you could check LL_GPIO_AF_EnableRemap_I2C1 for instance. You'll find it in ext/hal/st/stm32cube/stm32f1xx/drivers/include/stm32f1xx_ll_gpio.h

@mattzzw
Copy link
Author

mattzzw commented Jan 4, 2019

Thanks for pointing me to the helper functions, exactly what I was looking for.
I added AFIO clock enabling and I2C_1 remapping to pinmux.cand it works.

@mattzzw
Copy link
Author

mattzzw commented Feb 9, 2019

Anything else I can do to for this PR?

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Last comments.
Additionally, please rebase on master

@galak galak added the platform: STM32 ST Micro STM32 label Apr 26, 2019
@erwango
Copy link
Member

erwango commented Jul 26, 2019

@mattzzw , sorry this PR has been left unreviewed for a (long) while.
Can you rebase and check CI issue?

@zephyrproject-rtos zephyrproject-rtos deleted a comment from codecov-io Sep 5, 2019
@galak galak added the Stale PR label Sep 12, 2019
@galak
Copy link
Collaborator

galak commented Sep 12, 2019

Any plans to update?

@nashif nashif closed this Dec 19, 2019
@mattzzw mattzzw deleted the issue_12243 branch December 24, 2019 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build error with I2C driver for nucleo_f103rb: DT_ST_STM32_I2C_V1_40005400_BASE_ADDRESS' undeclared here
5 participants