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

S5JS100: remove idle hook in favour of Mbed-default idle hook #13978

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Nov 27, 2020

Summary of changes

Fix for S5JS100 mentioned in #13265

S5JS100's hal_sleep() implementation attaches an idle hook using rtos_attach_idle_hook(), but

  • rtos_attach_idle_hook() is reserved to be only used by the user-facing API Kernel::attach_idle_hook(). Conflicts will happen, if an application uses Kernel::attach_idle_hook() (which is a valid use case) - the application's idle hook will override S5JS100 hal_sleep()'s.
  • The S5JS100 idle hook (here) is just the Mbed OS default idle hook for non-Tickless (here).

This PR removes the S5JS100-specific idle hook in favour of the Mbed-default one. See discussion here.

Note: This is only a fix for S5JS100's usage of RTOS idle hook. The bare metal profile is not yet supported for this target, as some additionally clean-up/restructuring would be required.

Impact of changes

Target-specific rework, no noticeable impact on developers.

Migration actions required

None.

Documentation

None.


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

@ARMmbed/team-samsung @ARMmbed/mbed-os-core


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Nov 27, 2020
@ciarmcom ciarmcom requested review from a team November 27, 2020 18:30
@ciarmcom
Copy link
Member

@LDong-Arm, thank you for your changes.
@ARMmbed/team-samsung @ARMmbed/mbed-os-maintainers please review.

@mergify mergify bot added needs: CI and removed needs: review labels Nov 30, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Dec 1, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Dec 1, 2020

Jenkins CI Test : ❌ FAILED

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-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-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage
jenkins-ci/mbed-os-ci_greentea-test
jenkins-ci/mbed-os-ci_cmake-example-test
jenkins-ci/mbed-os-ci_cloud-client-pytest

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 2, 2020

CI restarted

@mbed-ci
Copy link

mbed-ci commented Dec 2, 2020

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_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-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_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest ✔️

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 2, 2020

@ARMmbed/team-samsung Can you review? We would like to merge this.

@0xc0170 0xc0170 merged commit 50e0981 into ARMmbed:master Dec 7, 2020
@mergify mergify bot removed the ready for merge label Dec 7, 2020
@mbedmain mbedmain added release-version: 6.6.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Dec 11, 2020
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.

6 participants