Skip to content
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

drivers: esp32: i2c: configure clock speed during runtime #72380

Merged
merged 1 commit into from
May 15, 2024

Conversation

maass-hamburg
Copy link
Collaborator

  • configure clock speed during runtime
  • implement get_config of the i2c api

@maass-hamburg
Copy link
Collaborator Author

ping @sylvioalves

@@ -262,13 +261,13 @@ static int i2c_esp32_recover(const struct device *dev)
return 0;
}

static void IRAM_ATTR i2c_esp32_configure_timeout(const struct device *dev)
static void IRAM_ATTR i2c_esp32_configure_timeout(const struct device *dev, uint32_t bitrate)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It sounds a bit strange to configure bit-rate in a configure_timeout function. I would either embed this into i2c_esp32_configure() or create a new i2c_esp32_configure_bitrate() function. How does it sound? I guess 1st option is a bit straighforward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fair point.
I now also moved the data mode part in a separate function, which is now only executed in the init, as it is set by the device tree and isn't changing during runtime.

@marekmatej
Copy link

@maass-hamburg can you please squash the commits into a single one?
Also, I think introducing the separate function i2c_esp32_configure_bitrate is better than hiding it under the timeout operation.

@maass-hamburg
Copy link
Collaborator Author

@maass-hamburg can you please squash the commits into a single one? Also, I think introducing the separate function i2c_esp32_configure_bitrate is better than hiding it under the timeout operation.

@marekmatej is done.

@marekmatej
Copy link

@maass-hamburg can you please squash the commits into a single one? Also, I think introducing the separate function i2c_esp32_configure_bitrate is better than hiding it under the timeout operation.

@marekmatej is done.

well, renaming the _timeout function to _bitrate wasn't what I had on my mind :) But in the end, it is doing its job, so I'm fine with the changes.

Copy link

@marekmatej marekmatej left a comment

Choose a reason for hiding this comment

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

Could you please update the commit message, so it has a better description of the changes, and more importantly how to use this new feature?

Be able to configure the clock frequency during runtime
using `i2c_configure()` and be able to use `i2c_get_config()`
of the i2c api for the esp32.

Signed-off-by: Fin Maaß <f.maass@vogl-electronic.com>
@aescolar aescolar merged commit 3b87a0b into zephyrproject-rtos:main May 15, 2024
21 checks passed
@maass-hamburg maass-hamburg deleted the esp32_i2c_config branch May 15, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants