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

Dev i2c common code #3324

Merged
merged 8 commits into from
Dec 9, 2016
Merged

Dev i2c common code #3324

merged 8 commits into from
Dec 9, 2016

Conversation

LMESTM
Copy link
Contributor

@LMESTM LMESTM commented Nov 24, 2016

Description

This PR makes most of i2C_api.c into one common file.

This required some changes to align all the main I2C functions to HAL layer which is common between families. Also the PR contains a few HAL update that were required to have all families (except FA) to the same level of functionalities. F1 I2C driver will be moved in the same way, when the official support for sequential transmit and receive will be available in HAL.

By doing this common code, we can then get the I2C_ASYNCH featuer for all common families.
So the PR also enables ASYNCH for all STM32 (except F1 ones for now).

Status

READY

Test report:
image

@bridadan
Copy link
Contributor

Changes look ok to me, I'll start the bots on this to make sure the build is OK. Do you have any test results or did you test out any of the I2C APIs manually?

Also @0xc0170, mind taking a look?

/morph test

@bridadan
Copy link
Contributor

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1165

All builds and test passed!

@LMESTM
Copy link
Contributor Author

LMESTM commented Nov 28, 2016

@bridadan I enclosed the tests results below the description.
I2C master slave asynch (MBED_A29) was tested automatically.
then I did manual tests of synchronous APIs with the IKS01A1 board which makes use of I2C sensors.

@bridadan
Copy link
Contributor

@LMESTM you're totally right about the test results, sorry about that. Currently suffering from some major jet lag, thanks for your patience 😄

@LMESTM
Copy link
Contributor Author

LMESTM commented Nov 28, 2016

@bridadan no pb ;-)

@mbed-bot
Copy link

[Build 1143]
FAILURE: Something went wrong when building and testing.

@LMESTM
Copy link
Contributor Author

LMESTM commented Nov 29, 2016

@bridadan Hi when it says something went wrong when building and testing : can you share what went wrong ?

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 29, 2016

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

@mbed-bot
Copy link

[Build 1147]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

@bridadan
Copy link
Contributor

@LMESTM Not sure what the failure was, but looks like it is resolved now. Could you please address the merge conflict?

This is prelim update before official V1.8.0 HAL to the needed HAL API
available as in F0 HAL which is using the same IP.
This is an alignement to F4 HAL as the same IP is used.
Next official HAL delivery update hall will include the same alignement.
Applying the same fix as in L1 and F4
In case of sequential transmit / receive, there is a need to:
- not use the reload option
- generate a new START on each new transaction

This applies to all HAL supporting the IP version V2.
Since most of the code in i2c_api.c is now relying on STM32 HAL, there
is now a possibility to make a common usage of this code accross families.

The IP version definition is introduced per family, to allow a switch of
functionnalities, especially the frequency management which differs.
BTw, we fix the F0 frequency settings at the same time.

F1 is managed for now as an exception as the HAL API for sequential transmit
/receive is not yet available (coming soon)
the I2C_ASYNCH feature is  added to all STM32 except
F1 family for now. Will be added when HAL update is done.
@LMESTM LMESTM force-pushed the dev_i2c_common_code branch from 7cc4934 to 0505a52 Compare November 30, 2016 07:26
@LMESTM
Copy link
Contributor Author

LMESTM commented Nov 30, 2016

@bridadan I just rebased and conflicts are resolved

@bridadan
Copy link
Contributor

bridadan commented Dec 1, 2016

Many thanks! @0xc0170 Could you review please? It'd be good to run the bots on this one more time before it comes in (due to the rebase).

@LMESTM
Copy link
Contributor Author

LMESTM commented Dec 1, 2016

@0xc0170 @bridadan
there has been an issue reported on MBED forum about the worng assert.
So I will actually add a basic commit change on top of this PR.

@@ -296,7 +296,7 @@ void i2c_frequency(i2c_t *obj, int hz)
struct i2c_s *obj_s = I2C_S(obj);
I2C_HandleTypeDef *handle = &(obj_s->handle);

MBED_ASSERT((hz > 0) && (hz <= 400000));
MBED_ASSERT((hz == 100000) || (hz == 400000) || (hz == 1000000));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this so strict, only 3 values accepted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are I2C supported frequencies
standard mode: 100 kbit/s, full speed: 400 kbit/s, fast mode: 1 mbit/s
Specific timings must be calculated for each frequency, and only timings for those values are implemented. Other requested frequencies may result in undefined behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking again more closely: you actually have a point. My remark is only valid to I2C_IP_VERSION_V2 where timings must be hard coded. For I2C_IP_VERSION_V1 the HAL layer makes dynamic calculation and also make the checks - so I could move this ASSERT to I2C_IP_VERSION_V2 case only - Is it ok if I make the change now ?

So make the assert to cover all possible values
Also assert applies only for I2C_IP_VERSION_V2.
Also in case of I2C_IP_VERSION_V1, the HAL makes the proper
checks and can dynamically scale the frequency in case of
intermediate value.
@LMESTM LMESTM force-pushed the dev_i2c_common_code branch from 533763d to 29b32b8 Compare December 1, 2016 14:20
@LMESTM
Copy link
Contributor Author

LMESTM commented Dec 5, 2016

@0xc0170 @bridadan
any further feedback or comments ?

@LMESTM
Copy link
Contributor Author

LMESTM commented Dec 6, 2016

ping ...

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 6, 2016

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 6, 2016

LGTM ! Waiting for CI to complete

@mbed-bot
Copy link

mbed-bot commented Dec 6, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1206

Example Build failed!

@LMESTM
Copy link
Contributor Author

LMESTM commented Dec 7, 2016

@0xc0170 the morph test seems to report a failure but following the link I only see a PASS directory. Is there an error to look at ?

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 7, 2016

@LMESTM looks to me failure with CI system, restarting it

/morph test

@mbed-bot
Copy link

mbed-bot commented Dec 7, 2016

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1215

All builds and test passed!

@bridadan
Copy link
Contributor

bridadan commented Dec 8, 2016

That first failure was because some repos failed to clone, looks like the second one was OK.

@0xc0170 0xc0170 merged commit 04f940d into ARMmbed:master Dec 9, 2016
ABOSTM added a commit to ABOSTM/mbed-os that referenced this pull request Aug 22, 2019
hugueskamba pushed a commit to evedon/mbed-os that referenced this pull request Aug 29, 2019
aisair pushed a commit to aisair/mbed that referenced this pull request Apr 30, 2024
Ports for Upcoming Targets

3241: Add support for FRDM-KW41 ARMmbed/mbed-os#3241
3291: Adding mbed enabled Maker board with NINA-B1 and EVA-M8Q ARMmbed/mbed-os#3291

Fixes and Changes

3062: TARGET_STM :USB device FS  ARMmbed/mbed-os#3062
3213: STM32: Refactor us_ticker.c + hal_tick.c files ARMmbed/mbed-os#3213
3288: Dev spi asynch l0l1 ARMmbed/mbed-os#3288
3289: Bug fix of initial value of interrupt edge in "gpio_irq_init" function. ARMmbed/mbed-os#3289
3302: STM32F4 AnalogIn - Clear VBATE and TSVREFE bits before configuring ADC channels ARMmbed/mbed-os#3302
3320: STM32 - Add ADC_VREF label ARMmbed/mbed-os#3320
3321: no HSE available by default for NUCLEO_L432KC ARMmbed/mbed-os#3321
3352: ublox eva nina - fix line endings ARMmbed/mbed-os#3352
3322: DISCO_L053C8 doesn't support LSE ARMmbed/mbed-os#3322
3345: STM32 - Remove TIM_IT_UPDATE flag in HAL_Suspend/ResumeTick functions ARMmbed/mbed-os#3345
3309: [NUC472/M453] Fix CI failed tests ARMmbed/mbed-os#3309
3157: [Silicon Labs] Adding support for EFR32MG1 wireless SoC ARMmbed/mbed-os#3157
3301: I2C - correct return values for write functions (docs) - part 1 ARMmbed/mbed-os#3301
3303: Fix #2956 #2939 #2957 #2959 #2960: Add HAL_DeInit function in gpio_irq destructor ARMmbed/mbed-os#3303
3304: STM32L476: no HSE is present in NUCLEO and DISCO boards ARMmbed/mbed-os#3304
3318: Register map changes for RevG ARMmbed/mbed-os#3318
3317: NUCLEO_F429ZI has integrated LSE ARMmbed/mbed-os#3317
3312: K64F: SPI Asynch API implementation ARMmbed/mbed-os#3312
3324: Dev i2c common code ARMmbed/mbed-os#3324
3369: Add CAN2 missing pins for connector CN12 ARMmbed/mbed-os#3369
3377: STM32 NUCLEO-L152RE Update system core clock to 32MHz ARMmbed/mbed-os#3377
3378: K66F: Enable LWIP feature ARMmbed/mbed-os#3378
3382: [MAX32620] Fixing serial readable function. ARMmbed/mbed-os#3382
3399: NUCLEO_F103RB - Add SERIAL_FC feature ARMmbed/mbed-os#3399
3409: STM32L1 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3409
3416: Renames i2c_api.c for STM32F1 targets to fix IAR exporter ARMmbed/mbed-os#3416
3348: Fix frequency function of CAN driver. ARMmbed/mbed-os#3348
3366: NUCLEO_F412ZG - Add new platform ARMmbed/mbed-os#3366
3379: STM32F0 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3379
3393: ISR register never re-evaluated in HAL_DMA_PollForTransfer for STM32F4 ARMmbed/mbed-os#3393
3408: STM32F7 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3408
3411: STM32L0 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3411
3424: STM32F4 - FIX to add the update of hdma->State variable ARMmbed/mbed-os#3424
3427: Fix stm i2c slave ARMmbed/mbed-os#3427
3429: Fix stm i2c fix init ARMmbed/mbed-os#3429
3434: [NUC472/M453] Fix stuck in lp_ticker_init and other updates ARMmbed/mbed-os#3434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants