Add pin speed controlling interface #11368

ua1arn · 2019-08-28T19:21:59Z

Description

Adding pin speed limit control for STM32-based targets. No other changes need, transparent for exiting code.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

ciarmcom · 2019-08-28T21:00:25Z

@ua1arn, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

jeromecoutant · 2019-08-29T07:15:34Z

This fixes #11362

@ARMmbed/team-st-mcd

0xc0170 · 2019-08-29T07:51:05Z

targets/TARGET_STM/pinmap.c

@@ -79,13 +80,21 @@ void pin_function(PinName pin, int data)
 #if defined (TARGET_STM32F1)
    if (mode == STM_PIN_OUTPUT) {
 #endif
-
+		switch (speed)
+		{


can you please fix formatting, follow the one in the file

switch (speed): case VALUE:

for 87-91 LL_ lines: they should move to be aligned with case (4 spaces to the right).

line 84 , move { to the line above (=attached).

line 87 : #if - preprocessor always at the beginning of the line (not aligned).

0xc0170 · 2019-08-29T07:51:48Z

Otherwise looks fine to me. Thanks for the fix!

Waiting for @ARMmbed/team-st-mcd review

0xc0170

Looks fine to me now, however if you can squash all commits into one, it would be good

ua1arn · 2019-09-02T09:45:28Z

Looks fine to me now, however if you can squash all commits into one, it would be good

Sorry, I can not doing this operation... This is a git command ?

LMESTM

Hi
just reviewed the proposal - looks good to me.
Just a proposal for renaming and few padding issues to solve I think

LMESTM · 2019-09-02T09:52:40Z

targets/TARGET_STM/PinNamesTypes.h

+                       (((CHAN)   & STM_PIN_CHAN_MASK) << STM_PIN_CHAN_SHIFT) |\
+                       (((INV)    & STM_PIN_INV_MASK) << STM_PIN_INV_SHIFT))
+
+#define STM_PIN_DEFINE_EXT2(FUNC_OD, PUPD, AFNUM, CHAN, INV, SPEEDV) \


Coud be STM_PIN_DEFINE_SPEED_EXT instead of _EXT2 ?

It's addressed to me?
change names up to you reasons,,

I cannot edit your Pull request. The idea is to all review together your proposal and ask you to take also our feedback into account, so that your code will be in master branch of mbed-os in the end and everyone can benefit for this improvement. It's just a minor change, so if you do it all your target code will also be compatible with it when it gets merged

Edits from maintainers are enabled for this pull request, Or, if I should make changes by own - what should I do?

Please do changes as requested!

LMESTM · 2019-09-02T09:53:32Z

targets/TARGET_STM/PinNamesTypes.h

@@ -90,15 +90,30 @@ extern "C" {
 #define STM_PIN_ANALOG_CONTROL(X)   (((X) >> STM_PIN_AN_CTRL_SHIFT) & STM_PIN_AN_CTRL_MASK)

 #define STM_PIN_DEFINE(FUNC_OD, PUPD, AFNUM)  ((int)(FUNC_OD) |\
-                          ((PUPD  & STM_PIN_PUPD_MASK) << STM_PIN_PUPD_SHIFT) |\
-                          ((AFNUM & STM_PIN_AFNUM_MASK) << STM_PIN_AFNUM_SHIFT))
+						  ((STM_PIN_SPEED_MASK   & STM_PIN_SPEED_MASK) << STM_PIN_SPEED_SHIFT) |\


seems like there is a padding issue

LMESTM · 2019-09-02T09:53:41Z

targets/TARGET_STM/PinNamesTypes.h

+                          (((AFNUM) & STM_PIN_AFNUM_MASK) << STM_PIN_AFNUM_SHIFT))
+
+#define STM_PIN_DEFINE_SPEED(FUNC_OD, PUPD, AFNUM, SPEEDV)  ((int)(FUNC_OD) |\
+						  (((SPEEDV)   & STM_PIN_SPEED_MASK) << STM_PIN_SPEED_SHIFT) |\


padding issue ?

LMESTM · 2019-09-02T09:53:53Z

targets/TARGET_STM/PinNamesTypes.h

+                       (((INV)    & STM_PIN_INV_MASK) << STM_PIN_INV_SHIFT))
+
+#define STM_PIN_DEFINE_EXT2(FUNC_OD, PUPD, AFNUM, CHAN, INV, SPEEDV) \
+                                            ((int)(FUNC_OD) |\


padding issue ?

I've noticed in one file tabs, so all changes in this PR should be checked and tabs replaced by spaces

Should I reformat original text too?

the best would be to only format what you are changing (it should be consistent, if there are issues, you can create an issue or new commit to address them).

files updated

LMESTM · 2019-09-02T09:57:14Z

@jeromecoutant will be worth a quick non-regression test like CI-shield test suite

LMESTM

Thanks a lot for your contribution !

0xc0170 · 2019-09-03T10:28:21Z

CI started

0xc0170 · 2019-09-03T12:58:54Z

Because of 5.14.0-rc1 CI jobs, we aborted CI job here. We will restart once 5.14.0rc1 is ready.

mbed-ci · 2019-09-03T12:59:32Z

Test run: FAILED

Summary: 4 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_mbed2-build-IAR
jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-09-04T07:36:19Z

CI restarted

mbed-ci · 2019-09-04T10:59:43Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

Add pin speed controlling interface

605fdd1

ciarmcom requested a review from a team August 28, 2019 21:00

ciarmcom added the needs: review label Aug 28, 2019

0xc0170 requested a review from a team August 29, 2019 07:49

0xc0170 requested changes Aug 29, 2019

View reviewed changes

dolphin\gena and others added 6 commits September 2, 2019 10:08

mistype fix

75c17ce

formatting issue

05fc0f5

fix mistypes

9043330

formatting

58ca130

ST pinmap: Fix the style

b44fbfe

ST pinmap: remove endif mistype

8add87a

0xc0170 reviewed Sep 2, 2019

View reviewed changes

LMESTM reviewed Sep 2, 2019

View reviewed changes

0xc0170 removed the needs: review label Sep 2, 2019

0xc0170 added the needs: work label Sep 2, 2019

dolphin\gena added 4 commits September 2, 2019 14:02

rename macro

1379009

indents correction

a684f43

bad formatting correction

2a13490

formatting

afc79cf

LMESTM approved these changes Sep 3, 2019

View reviewed changes

0xc0170 approved these changes Sep 3, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Sep 3, 2019

0xc0170 added ready for merge release-version: 5.14.1 and removed needs: CI labels Sep 4, 2019

0xc0170 merged commit 7b06ce5 into ARMmbed:master Sep 4, 2019

0xc0170 removed the ready for merge label Sep 4, 2019

jeromecoutant mentioned this pull request Oct 11, 2019

Pin speed control #11362

Closed

Add pin speed controlling interface #11368

Add pin speed controlling interface #11368

Uh oh!

Conversation

ua1arn commented Aug 28, 2019

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Aug 28, 2019

Uh oh!

jeromecoutant commented Aug 29, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

ua1arn commented Sep 2, 2019

Uh oh!

LMESTM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ua1arn Sep 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LMESTM commented Sep 2, 2019

Uh oh!

LMESTM left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Sep 3, 2019

Uh oh!

0xc0170 commented Sep 3, 2019

Uh oh!

mbed-ci commented Sep 3, 2019

Uh oh!

0xc0170 commented Sep 4, 2019

Uh oh!

mbed-ci commented Sep 4, 2019

Uh oh!

Uh oh!

0xc0170 commented Aug 29, 2019 •

edited

Loading

ua1arn Sep 2, 2019 •

edited

Loading