driver/i2c: STM32: I2C performance issue solved. #14808

affrinpinhero-2356 · 2021-06-21T14:47:15Z

Summary of changes

This commit solves issue related to i2c driver performance.
With this commit delay in read write when using i2c timing
algorithm is solved. Used flag mechanism which will check
tim reg value and hz passed.
Depends on #14776
since not merged.

Impact of changes

Delay in I2C read write while using i2c timing algorithm is solved.

Migration actions required

Documentation

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

ciarmcom · 2021-06-21T15:00:13Z

@affrinpinhero-2356, thank you for your changes.
@ARMmbed/team-st-mcd @ARMmbed/mbed-os-maintainers please review.

jeromecoutant · 2021-06-22T08:01:33Z

Hi
I will make the final review after #14776

But I would expect more in this PR.
In targets.json, I imagine 3 new config at MCU_STM32 level:

I2C-timing-value-standard-mode
I2C-timing-value-fast-mode
I2C-timing-value-fast-plus-mode
With no value set

Then in each i2c_device.h; you could set something like:

#ifdef MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#define I2C_TIMING_VALUE_STANDARD_MODE MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#else
#define I2C_TIMING_VALUE_STANDARD_MODE <default value>
#endif

So for default user, ALGO is disabled, and no explicit timing values are defined,
=> everything is working as for now

If user makes an application that changes dynamically the frequency settings
=> ALGO macro should be defined

If user makes an application that changes the default settings
=> he can keep the ALGO macro disabled, and only change the default timing value

Regards,

jeromecoutant · 2021-06-22T08:13:46Z

And add few lines in https://github.com/ARMmbed/mbed-os/tree/master/targets/TARGET_STM#st-specific-implementation
for documentation

ciarmcom · 2021-06-25T07:50:26Z

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/Team-ST-MCD, @ARMmbed/mbed-os-maintainers, please complete review of the changes to move the PR forward. Thank you for your contributions.

mergify · 2021-06-29T09:31:17Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

0xc0170 · 2021-06-29T11:51:16Z

Depends on #14776

It was merged, please rebase

targets/TARGET_STM/i2c_api.c

affrinpinhero-2356 · 2021-07-01T09:17:06Z

@jeromecoutant

Hi
I will make the final review after #14776

But I would expect more in this PR.
In targets.json, I imagine 3 new config at MCU_STM32 level:

I2C-timing-value-standard-mode

I2C-timing-value-fast-mode

I2C-timing-value-fast-plus-mode
With no value set

Then in each i2c_device.h; you could set something like:
#ifdef MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#define I2C_TIMING_VALUE_STANDARD_MODE MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#else
#define I2C_TIMING_VALUE_STANDARD_MODE <default value>
#endif
So for default user, ALGO is disabled, and no explicit timing values are defined,
=> everything is working as for now

If user makes an application that changes dynamically the frequency settings
=> ALGO macro should be defined

If user makes an application that changes the default settings
=> he can keep the ALGO macro disabled, and only change the default timing value

Regards,

For some targets, we have more system clock values defined.
https://github.com/ARMmbed/mbed-os/pull/14808/files#diff-3a9aa9e0acd297be19962adede5965c6aaaa1fbcfc70fa4f2f2cf529e64c9da6R43-R51

jeromecoutant · 2021-07-01T10:05:10Z

#ifdef MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#define I2C_TIMING_VALUE_STANDARD_MODE MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#else
#define I2C_TIMING_VALUE_STANDARD_MODE <default value>
#endif
For some targets, we have more system clock values defined.
https://github.com/ARMmbed/mbed-os/pull/14808/files#diff-3a9aa9e0acd297be19962adede5965c6aaaa1fbcfc70fa4f2f2cf529e64c9da6R43-R51

For STM32F3 for ex, I could propose?

#ifdef MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#define I2C_TIMING_VALUE_STANDARD_MODE MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#else
#  if !defined(RCC_CFGR_PLLSRC_HSI_PREDIV)
#    if ((CLOCK_SOURCE) == USE_PLL_HSI)
#define I2C_TIMING_VALUE_STANDARD_MODE <default value>
#    endif
#  endif
#endif

prasannaven · 2021-07-01T13:01:19Z

#ifdef MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#define I2C_TIMING_VALUE_STANDARD_MODE MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#else
#define I2C_TIMING_VALUE_STANDARD_MODE <default value>
#endif
For some targets, we have more system clock values defined.
https://github.com/ARMmbed/mbed-os/pull/14808/files#diff-3a9aa9e0acd297be19962adede5965c6aaaa1fbcfc70fa4f2f2cf529e64c9da6R43-R51
For STM32F3 for ex, I could propose?
#ifdef MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#define I2C_TIMING_VALUE_STANDARD_MODE MBED_CONF_TARGET_I2C_TIMING_VALUE_STANDARD_MODE
#else
#  if !defined(RCC_CFGR_PLLSRC_HSI_PREDIV)
#    if ((CLOCK_SOURCE) == USE_PLL_HSI)
#define I2C_TIMING_VALUE_STANDARD_MODE <default value>
#    endif
#  endif
#endif

@jeromecoutant Can we take this update with separate ticket ?

jeromecoutant

Non regression tests are failing with F303ZE and L476RG

affrinpinhero-2356 · 2021-07-01T16:54:47Z

Non regression tests are failing with F303ZE and L476RG

Let me check the same. Can you paste the error.

This commit solves issue related to i2c driver performance. With this commit delay in read write when using i2c timing algorithm is solved. Used flag mechanism which will check tim reg value and hz passed. Signed-off-by: Affrin Pinhero <[email protected]>

jeromecoutant · 2021-07-02T08:04:29Z

Non regression tests are failing with F303ZE and L476RG

In i2c_get_timing:
#if defined (I2C_PCLK_32M)
...
#elif defined(I2C_PCLK_64M)
#elif defined (I2C_PCLK_72M)
...

=> several I2C_PCLK_xxx can't be defined as for F3 or L4

Proposition:
#if defined (I2C_PCLK_32M)
#endif
#if defined(I2C_PCLK_64M)
#endif
#if defined (I2C_PCLK_72M)
#endif

affrinpinhero-2356 · 2021-07-02T08:47:48Z

Non regression tests are failing with F303ZE and L476RG

In i2c_get_timing:
#if defined (I2C_PCLK_32M)
...
#elif defined(I2C_PCLK_64M)
#elif defined (I2C_PCLK_72M)
...

=> several I2C_PCLK_xxx can't be defined as for F3 or L4

Proposition:
#if defined (I2C_PCLK_32M)
#endif
#if defined(I2C_PCLK_64M)
#endif
#if defined (I2C_PCLK_72M)
#endif

Modified already. Does it still cause errors?

https://github.com/ARMmbed/mbed-os/pull/14808/files#diff-8669c41247ba3cd273836b6a4c4376b94293f4852ac54e47cd3c01b6c9c43c7aR1778-R1796
https://github.com/ARMmbed/mbed-os/pull/14808/files#diff-8669c41247ba3cd273836b6a4c4376b94293f4852ac54e47cd3c01b6c9c43c7aR1872-R1890
https://github.com/ARMmbed/mbed-os/pull/14808/files#diff-8669c41247ba3cd273836b6a4c4376b94293f4852ac54e47cd3c01b6c9c43c7aR1910-R1928

affrinpinhero-2356 · 2021-07-05T07:32:49Z

Is this good to start CI?

affrinpinhero-2356 · 2021-07-06T10:28:22Z

@jeromecoutant is this good to start CI?

0xc0170 · 2021-07-06T11:30:49Z

CI started

mbed-ci · 2021-07-06T12:04:02Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

0xc0170

Just readme update to fix grammar. sorry for late request, I've noticed it just now

targets/TARGET_STM/README.md

This commit modifies readme file. Descption for using I2C timing algorithm and how to enable and disable included. Signed-off-by: Affrin Pinhero <[email protected]>

Pull request has been modified.

0xc0170 · 2021-07-08T11:34:34Z

CI started

mbed-ci · 2021-07-08T12:29:09Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

ciarmcom added the release-type: feature label Jun 21, 2021

ciarmcom requested review from a team June 21, 2021 15:00

ciarmcom added needs: review devices: st labels Jun 21, 2021

ciarmcom added the stale Stale Pull Request label Jun 25, 2021

ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Jun 25, 2021

mergify bot added the needs: work label Jun 29, 2021

mergify bot removed the needs: review label Jun 29, 2021

ciarmcom removed the stale Stale Pull Request label Jun 29, 2021

LMESTM reviewed Jun 29, 2021

View reviewed changes

targets/TARGET_STM/i2c_api.c Outdated Show resolved Hide resolved

affrinpinhero-2356 force-pushed the i2cTimingPerformanceSolved branch from 6bdf48a to 98dde59 Compare July 1, 2021 06:55

affrinpinhero-2356 requested review from LMESTM and removed request for a team July 1, 2021 15:41

jeromecoutant suggested changes Jul 1, 2021

View reviewed changes

affrinpinhero-2356 force-pushed the i2cTimingPerformanceSolved branch from a63daf8 to cb78629 Compare July 2, 2021 05:28

jeromecoutant approved these changes Jul 2, 2021

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Jul 6, 2021

0xc0170 previously requested changes Jul 6, 2021

View reviewed changes

targets/TARGET_STM/README.md Outdated Show resolved Hide resolved

targets/TARGET_STM/README.md Outdated Show resolved Hide resolved

mergify bot added needs: work and removed needs: CI labels Jul 6, 2021

I2C: STM32: Upating documentation for I2C timing Algorithm

b559cec

This commit modifies readme file. Descption for using I2C timing algorithm and how to enable and disable included. Signed-off-by: Affrin Pinhero <[email protected]>

affrinpinhero-2356 force-pushed the i2cTimingPerformanceSolved branch from cb78629 to b559cec Compare July 6, 2021 14:20

affrinpinhero-2356 requested a review from 0xc0170 July 6, 2021 16:46

0xc0170 approved these changes Jul 8, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Jul 8, 2021

mergify bot added ready for merge and removed needs: CI labels Jul 8, 2021

0xc0170 added release-type: patch Indentifies a PR as containing just a patch and removed release-type: feature labels Jul 8, 2021

0xc0170 merged commit 1d5d3b0 into ARMmbed:master Jul 8, 2021

mergify bot removed the ready for merge label Jul 8, 2021

mbedmain added release-version: 6.13.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jul 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

driver/i2c: STM32: I2C performance issue solved. #14808

driver/i2c: STM32: I2C performance issue solved. #14808

affrinpinhero-2356 commented Jun 21, 2021 •

edited by 0xc0170

Loading

ciarmcom commented Jun 21, 2021

jeromecoutant commented Jun 22, 2021

jeromecoutant commented Jun 22, 2021

ciarmcom commented Jun 25, 2021

mergify bot commented Jun 29, 2021

0xc0170 commented Jun 29, 2021

affrinpinhero-2356 commented Jul 1, 2021 •

edited

Loading

jeromecoutant commented Jul 1, 2021

prasannaven commented Jul 1, 2021

jeromecoutant left a comment

affrinpinhero-2356 commented Jul 1, 2021

jeromecoutant commented Jul 2, 2021

affrinpinhero-2356 commented Jul 2, 2021

affrinpinhero-2356 commented Jul 5, 2021

affrinpinhero-2356 commented Jul 6, 2021

0xc0170 commented Jul 6, 2021

mbed-ci commented Jul 6, 2021

0xc0170 left a comment

0xc0170 commented Jul 8, 2021

mbed-ci commented Jul 8, 2021

driver/i2c: STM32: I2C performance issue solved. #14808

driver/i2c: STM32: I2C performance issue solved. #14808

Conversation

affrinpinhero-2356 commented Jun 21, 2021 • edited by 0xc0170 Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Jun 21, 2021

jeromecoutant commented Jun 22, 2021

jeromecoutant commented Jun 22, 2021

ciarmcom commented Jun 25, 2021

mergify bot commented Jun 29, 2021

0xc0170 commented Jun 29, 2021

affrinpinhero-2356 commented Jul 1, 2021 • edited Loading

jeromecoutant commented Jul 1, 2021

prasannaven commented Jul 1, 2021

jeromecoutant left a comment

Choose a reason for hiding this comment

affrinpinhero-2356 commented Jul 1, 2021

jeromecoutant commented Jul 2, 2021

affrinpinhero-2356 commented Jul 2, 2021

affrinpinhero-2356 commented Jul 5, 2021

affrinpinhero-2356 commented Jul 6, 2021

0xc0170 commented Jul 6, 2021

mbed-ci commented Jul 6, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

0xc0170 left a comment

Choose a reason for hiding this comment

0xc0170 commented Jul 8, 2021

mbed-ci commented Jul 8, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

affrinpinhero-2356 commented Jun 21, 2021 •

edited by 0xc0170

Loading

affrinpinhero-2356 commented Jul 1, 2021 •

edited

Loading