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

STM32 EMAC configuration update #11536

Merged
merged 3 commits into from
Oct 23, 2019
Merged

Conversation

jeromecoutant
Copy link
Collaborator

Description

Hi

Issue comes from discussion in #11506 (comment)

It looks not easy to remove the default Ethernet capability for 1 board...

So

  • TARGET_STM_EMAC directory has been renamed into TARGET_STM
    (which doesn't need any specific extra label addition)
  • network-default-interface-type is set in the mbed_lib json file

Now removing the EMAC device_has feature is the only parameter to set or not

Pull request type

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

Reviewers

@ARMmbed/team-st-mcd
@ARMmbed/team-ublox

Release Notes

@ciarmcom ciarmcom requested review from a team September 20, 2019 09:00
@ciarmcom
Copy link
Member

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

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 24, 2019

Refactor

This touches only target code, shall it go to 5.15 rather?

@jeromecoutant
Copy link
Collaborator Author

shall it go to 5.15 rather?

I prefer next 5.14 ...

@SeppoTakalo
Copy link
Contributor

This feels more like a workaround, than fix.

It should be found out why those buffers mentioned in #11506 are not dropped by the linker.

@mbed-ci
Copy link

mbed-ci commented Sep 24, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 1, 2019

It should be found out why those buffers mentioned in #11506 are not dropped by the linker.

@jeromecoutant Any update?

@jeromecoutant
Copy link
Collaborator Author

Build issue should be corrected now.

Compilation ex with the echo test:

  • NUCLEO_F429ZI default configuration:
    | name | target | toolchain | static_ram | total_flash |
    | echo | NUCLEO_F429ZI | ARMC6 | 26967 | 65556 |

  • NUCLEO_F429ZI with local configuration "target.device_has_remove": ["EMAC"] :
    | echo | NUCLEO_F429ZI | ARMC6 | 11126 | 55085 |

@jeromecoutant
Copy link
Collaborator Author

Thanks to start CI

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 18, 2019

It should be found out why those buffers mentioned in #11506 are not dropped by the linker.

@jeromecoutant Is this fixing 11506 or ? It really does not look like a proper fix. However if this is a simplification work, then should be fine.

@jeromecoutant
Copy link
Collaborator Author

I don't plan any more work on this topic...

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 21, 2019

This feels more like a workaround, than fix.

It should be found out why those buffers mentioned in #11506 are not dropped by the linker.

@jeromecoutant This does not fix 11506 completely, so that issue would still be opened (it hides it that I do not like much as stated in the quote above)..
The changes look good to me as they are here, removing STM EMAC label

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 21, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Oct 21, 2019

Test run: SUCCESS

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

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 21, 2019

@SeppoTakalo Happy with this (considering this is not the fix for the issue, but rather refactor) ?

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 23, 2019

@jeromecoutant This does not fix 11506 completely, so that issue would still be opened (it hides it that I do not like much as stated in the quote above)..

Taking this in as it is, #11506 stays opened

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.

None yet

6 participants