Add Samsung Exynos i S111 target #12106

andrewc-arm · 2019-12-16T03:19:31Z

Original Issue Case: https://github.com/ARMmbed/Samsung_projects/issues/63
Previous PR try: #10170

Description

Adding a new target of HW development kit using Samsung Exynos i S111 module to Mbed-OS.
This will widen the HW choices of Mbed-OS enabled NB-IoT, GNSS and Security (eFuse, AES, SHA-2, PKA, Secure Storage, Security Sub-System, PUF) modules.
Target Name: S5JS100

Main author: Jihoon Park jh6186.park@samsung.com
Co-authored-by: Ivan Galkin ivan.galkin@samsung.com
Co-authored-by: Seokwon Lee swon.lee@samsung.com
Co-authored-by: Zhizhe Zhu zhizhe.zhu@samsung.com
Co-authored-by: Xinyi Zhao xinyi.zhao@samsung.com

Summary of changes

Adding a new target of S5JS100 platform using S111 NB-IoT Cortex-M7 chip.

        modified:   platform/mbed_lib.json
        modified:   targets/targets.json
        modified:   tools/export/iar/iar_definitions.json
        added:   features/mbedtls/targets/TARGET_Samsung/
        added:   targets/TARGET_Samsung/

Impact of changes

There should not be any impact to Mbed-OS or other targets.

Migration actions required

None.

Documentation

Arm internal (for now): https://github.com/ARMmbed/Samsung_projects/wiki

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

Full build report and Greentea test report: log - 2019-12-16-02.7z.zip of this submit (9fa4279)

Reviewers

ciarmcom · 2019-12-16T04:00:26Z

@andrewc-arm, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-crypto please review.

andrewc-arm · 2019-12-16T04:56:23Z

Hi,

The travis-ci test shows this failure.

    def test_device_name():
        """Assert device name is in a pack"""
        cache = Cache(True, True)
        named_targets = (target for target in TARGETS if
                         hasattr(target, "device_name"))
        for target in named_targets:
>           assert target.device_name in cache.index,\
                ("Target %s contains invalid device_name %s" %
                 (target.name, target.device_name))
E           AssertionError: Target S5JS100 contains invalid device_name S5JS100
E           assert 'S5JS100' in <tools.arm_pack_manager._CacheLookup object at 0x7fc8c0e2b810>
E            +  where 'S5JS100' = Target(name=u'S5JS100', json_data={u'Target': OrderedDict([(u'core', None), (u...home/travis/build/ARMmbed/mbed-os/tools/targets/../../targets/targets.json')])).device_name
E            +  and   <tools.arm_pack_manager._CacheLookup object at 0x7fc8c0e2b810> = <tools.arm_pack_manager.Cache object at 0x7fc8c46dfc10>.index

There is CMSIS pack ready as I mentioned in the description. How can we resolve this?

0xc0170 · 2019-12-16T10:31:24Z

@andrewc-arm cmsis pack has not yet been published - public availability ?

andrewc-arm · 2019-12-17T00:50:37Z

@andrewc-arm cmsis pack has not yet been published - public availability ?

No. Not yet. It's under legal check now. I remember some code tweak to bypass the CMSIS pack check and I will try to do so for my business hour. Later when the S5JS100 CMSIS pack is public, I will remove the bypass.

(CC: @jh6186)

andrewc-arm · 2019-12-17T02:31:56Z

Hi,
I just removed the connection between Mbed-OS target S5JS100 and CMSIS pack. The connection was device_name and it's removed via d32a055. (CC: @jh6186 )

For others, here is the document about device_name.
https://os.mbed.com/docs/mbed-os/v5.14/reference/adding-and-configuring-targets.html

The "device_name" attribute it targets.json maps from a target in Mbed OS to a device in a CMSIS Pack. To support IAR and uVision exports for your target, you must add a "device_name" field in targets.json containing this key.

Later, when the S5JS100 CMSIS pack is public, we will revive the connection so that IAR/uVision exports can be done seamlessly.

andrewc-arm · 2019-12-18T11:27:26Z

@ARMmbed/mbed-os-maintainers
Could you please help to get this PR going? I see that the last test is held on continuous-integration/jenkins/pr-head. I thought I heard this test is sometimes manually triggered.
FYI. There are quite many tasks depending on this PR. Thank you!
(CC: @TuomoHautamaki , @ohadhawk )

kjbracey · 2019-12-19T07:28:52Z

Review still pending, but I'll trigger a CI for an initial check, as capacity is available.

mbed-ci · 2019-12-19T08:18:52Z

Test run: SUCCESS

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

andrewc-arm · 2019-12-19T08:21:40Z

Hi,
The related engineering team just found a corner case with timer driver and I will need to push in with the code change and the new Greentea test report. Please hold merging for about a day. Thanks!

adbridge · 2019-12-19T12:04:02Z

Hi,
The related engineering team just found a corner case with timer driver and I will need to push in with the code change and the new Greentea test report. Please hold merging for about a day. Thanks!

Hi @andrewc-arm this will also need a proper review before it can be accepted. With 102 files added this may not make it in time for Xmas I'm afraid as a lot of people are already going off for Xmas. Will see what we can do.

bulislaw

Couple of questions, but it looks good.

targets/targets.json

targets/TARGET_Samsung/TARGET_SIDK_S5JS100/device/TOOLCHAIN_IAR/sidk_s5js100.icf

targets/TARGET_Samsung/TARGET_SIDK_S5JS100/device/s5js100_systemreset.c

targets/TARGET_Samsung/TARGET_SIDK_S5JS100/mbed_sdk_init.c

targets/TARGET_Samsung/TARGET_SIDK_S5JS100/port_api.c

targets/targets.json

adbridge

The files reviewed so far do not meet the arm coding guidelines. Please check that all files that implement Mbed OS functionality (rather than the low level Samsung SDK) are compliant and update the PR accordingly. I will then continue the review. Thanks.

features/mbedtls/targets/TARGET_Samsung/mbedtls_device.h

features/mbedtls/targets/TARGET_Samsung/sha/sha256_alt.c

features/mbedtls/targets/TARGET_Samsung/sha/sha256_alt.h

features/mbedtls/targets/TARGET_Samsung/sha/sha256_alt.c

targets/TARGET_Samsung/TARGET_SIDK_S5JS100/PeripheralNames.h

andrewc-arm · 2019-12-20T06:22:10Z

Hi, @ARMmbed/mbed-os-maintainers and @adbridge

this may not make it in time for Xmas I'm afraid as a lot of people are already going off for Xmas. Will see what we can do.

I just found out from the Samsung team that this code is also under review by the Samsung code release process. And it has not yet passed their internal process yet. Please understand that this joint effort is the first time between two companies. So, could you kindly hold off the actual merging process until we get the OK sign from all the stakeholders including the Samsung team? This means this process may take until the next year January, so we are not in a hurry.

adbridge · 2019-12-23T12:00:59Z

@andrewc-arm no problem at all, It's is good to see the collaboration coming along :)

0xc0170

we will review once reviews are addressed. The files should be cleaned to remove dead code or comments with questions like "What is this for" (if they are not answered, please ask here, will try to answer all of them)

features/mbedtls/targets/TARGET_Samsung/mbedtls_device.h

adbridge · 2020-01-06T11:35:10Z

@andrewc-arm looks like you also need a rebase

andrewc-arm · 2020-01-07T03:26:48Z

@andrewc-arm looks like you also need a rebase

Thanks for pointing it out, @adbridge . I just finished a rebase and it's running the full Greentea test suite again.

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

…eaders.

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

andrewc-arm · 2020-01-22T05:42:29Z

Hi, @ARMmbed/mbed-os-maintainers

Could you please consider merging this code to mbed-os master?
I just rebased and got the full Greentea test pass.
log - 2020-01-22.7z.zip

I do not wish to rebase and Greentea test everyday just because the admin didn't progress. 😃

adbridge · 2020-01-22T12:28:57Z

@Patater @bulislaw @mark-edgeworth could you all just approve this if you are now happy with the updates? Thanks

mark-edgeworth

Looks ok from a tools point of view.

Patater

Still LGTM

adbridge · 2020-01-22T14:57:16Z

CI started

mbed-ci · 2020-01-22T15:50:08Z

Test run: SUCCESS

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

andrewc-arm · 2020-01-23T08:58:26Z

Hi, @adbridge and @ARMmbed/mbed-os-maintainers
Thank you so much for leading this to this point.
Does this mean it will be merged soon? BTW, there are other work items waiting for this merging. 😃
Should we just wait or are there remaining actions to the final merging?
It would be nice if we don't have to go through another round of rebase, Greentea and CI. 😋

mergify · 2020-01-23T10:52:50Z

This PR does not contain release version label after merging.

LDong-Arm · 2020-06-17T13:45:41Z

targets/TARGET_Samsung/TARGET_SIDK_S5JS100/sleep.c

+static void s5js100_idle_hook(void)
+{
+    core_util_critical_section_enter();
+    sleep();
+    core_util_critical_section_exit();
+}
+
+static void set_sleep_policy(void)
+{
+    rtos_attach_idle_hook(&s5js100_idle_hook);


Sorry to raise this since the PR is merged already, but I'm wondering if we can drop this idle hook and use the default one?

The Mbed OS default idle hook supports Tickless:

mbed-os/rtos/source/TARGET_CORTEX/mbed_rtx_idle.cpp

Lines 133 to 154 in b1629b7

static void default_idle_hook(void)

{

rtos::Kernel::Clock::duration_u32 ticks_to_sleep{osKernelSuspend()};

// osKernelSuspend will call OS_Tick_Disable, cancelling the tick, which frees

// up the os timer for the timed sleep

rtos::Kernel::Clock::duration_u32 ticks_slept = mbed::internal::do_timed_sleep_relative_to_acknowledged_ticks(ticks_to_sleep, rtos_event_pending);

MBED_ASSERT(ticks_slept < rtos::Kernel::wait_for_u32_max);

osKernelResume(ticks_slept.count());

}

#else // MBED_TICKLESS

static void default_idle_hook(void)

{

// critical section to complete sleep with locked deepsleep

core_util_critical_section_enter();

sleep_manager_lock_deep_sleep();

sleep();

sleep_manager_unlock_deep_sleep();

core_util_critical_section_exit();

}

Also, calling rtos_attach_idle_hook here means the hal_sleep() implementation can't compile with the bare metal profile.

@andrewc-arm @kjbracey-arm

@vince-zeng can you take a look together with @kjbracey-arm ?
@LDong-Arm , FYI andrewc-arm no longer works for Arm.

I have forward the info to Samsung team and please them help to check. Thanks!

In default_idle_hook(), the only different with s5js100_idle_hook() is that the sleep_manager_lock_deep_sleep_internal() is called.
But I am not familiar with this function, could you please share about this?

@LDong-Arm Can you create an issue with details and we can discuss it there and fix?

#13265 created

ciarmcom requested review from a team December 16, 2019 04:00

ciarmcom added the needs: review label Dec 16, 2019

andrewc-arm force-pushed the pr-S5JS100-05 branch from c8709a4 to 9fa4279 Compare December 16, 2019 06:15

0xc0170 added needs: work and removed needs: review labels Dec 16, 2019

adbridge added the Release review required label Dec 19, 2019

bulislaw reviewed Dec 19, 2019

View reviewed changes

targets/targets.json Show resolved Hide resolved

adbridge previously requested changes Dec 19, 2019

View reviewed changes

0xc0170 requested changes Jan 2, 2020

View reviewed changes

features/mbedtls/targets/TARGET_Samsung/mbedtls_device.h Outdated Show resolved Hide resolved

andrewc-arm force-pushed the pr-S5JS100-05 branch from 905e655 to bb0abc8 Compare January 7, 2020 03:17

0xc0170 requested review from 0xc0170 and adbridge January 7, 2020 10:21

jh6186 and others added 10 commits January 22, 2020 14:40

Update SSS header contents

adbc391

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

Minor astyle change.

77457e8

fix header for samsung spi driver

3abe45f

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

Adding in SPDX Apache 2.0 for the Apache 2 declared Samsung license h…

f9c4a51

…eaders.

Default use BEDTLS_SHA512_SMALLER for SW fallback

4460d26

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

fix typo 'modifyh'

d5bb27a

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

fix typo 'modifyh'

4ef74ef

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

Tidy up comments

3ba075b

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

comments the performance of TRNG

8b0fb5f

Signed-off-by: PARKJIHOON <jh6186.park@samsung.com>

andrewc-arm force-pushed the pr-S5JS100-05 branch from c4d7fdf to 8b0fb5f Compare January 22, 2020 05:41

mark-edgeworth approved these changes Jan 22, 2020

View reviewed changes

Patater approved these changes Jan 22, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Jan 22, 2020

bulislaw approved these changes Jan 22, 2020

View reviewed changes

mergify bot added ready for merge and removed needs: CI labels Jan 22, 2020

adbridge merged commit 24ccfab into ARMmbed:master Jan 23, 2020

adbridge removed the ready for merge label Jan 23, 2020

pea-pod mentioned this pull request Jan 23, 2020

Add new target: NUCLEO_L452RE-P #12286

Merged

bulislaw added release-version: 5.15.1 and removed Release review required labels Feb 4, 2020

LDong-Arm reviewed Jun 17, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Samsung Exynos i S111 target #12106

Add Samsung Exynos i S111 target #12106

andrewc-arm commented Dec 16, 2019 •

edited

Loading

ciarmcom commented Dec 16, 2019

andrewc-arm commented Dec 16, 2019

0xc0170 commented Dec 16, 2019

andrewc-arm commented Dec 17, 2019 •

edited

Loading

andrewc-arm commented Dec 17, 2019

andrewc-arm commented Dec 18, 2019

kjbracey commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

andrewc-arm commented Dec 19, 2019

adbridge commented Dec 19, 2019

bulislaw left a comment

adbridge left a comment

andrewc-arm commented Dec 20, 2019

adbridge commented Dec 23, 2019

0xc0170 left a comment

adbridge commented Jan 6, 2020

andrewc-arm commented Jan 7, 2020

andrewc-arm commented Jan 22, 2020 •

edited

Loading

adbridge commented Jan 22, 2020

mark-edgeworth left a comment

Patater left a comment

adbridge commented Jan 22, 2020

mbed-ci commented Jan 22, 2020

andrewc-arm commented Jan 23, 2020

mergify bot commented Jan 23, 2020

LDong-Arm Jun 17, 2020

ohadhawk Jun 17, 2020

vince-zeng Jun 18, 2020

zxytddd Jun 19, 2020

0xc0170 Jun 19, 2020

LDong-Arm Jul 10, 2020

	static void default_idle_hook(void)
	{
	rtos::Kernel::Clock::duration_u32 ticks_to_sleep{osKernelSuspend()};
	// osKernelSuspend will call OS_Tick_Disable, cancelling the tick, which frees
	// up the os timer for the timed sleep
	rtos::Kernel::Clock::duration_u32 ticks_slept = mbed::internal::do_timed_sleep_relative_to_acknowledged_ticks(ticks_to_sleep, rtos_event_pending);
	MBED_ASSERT(ticks_slept < rtos::Kernel::wait_for_u32_max);
	osKernelResume(ticks_slept.count());
	}


	#else // MBED_TICKLESS

	static void default_idle_hook(void)
	{
	// critical section to complete sleep with locked deepsleep
	core_util_critical_section_enter();
	sleep_manager_lock_deep_sleep();
	sleep();
	sleep_manager_unlock_deep_sleep();
	core_util_critical_section_exit();
	}

Add Samsung Exynos i S111 target #12106

Add Samsung Exynos i S111 target #12106

Conversation

andrewc-arm commented Dec 16, 2019 • edited Loading

Description

Summary of changes

mbed-os-tool PR for supporting new target: S5JS100

pyOCD PR for supporting new target: S5JS100

CMSIS pack for the new target: S5JS100

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Dec 16, 2019

andrewc-arm commented Dec 16, 2019

0xc0170 commented Dec 16, 2019

andrewc-arm commented Dec 17, 2019 • edited Loading

andrewc-arm commented Dec 17, 2019

andrewc-arm commented Dec 18, 2019

kjbracey commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

andrewc-arm commented Dec 19, 2019

adbridge commented Dec 19, 2019

bulislaw left a comment

Choose a reason for hiding this comment

adbridge left a comment

Choose a reason for hiding this comment

andrewc-arm commented Dec 20, 2019

adbridge commented Dec 23, 2019

0xc0170 left a comment

Choose a reason for hiding this comment

adbridge commented Jan 6, 2020

andrewc-arm commented Jan 7, 2020

andrewc-arm commented Jan 22, 2020 • edited Loading

adbridge commented Jan 22, 2020

mark-edgeworth left a comment

Choose a reason for hiding this comment

Patater left a comment

Choose a reason for hiding this comment

adbridge commented Jan 22, 2020

mbed-ci commented Jan 22, 2020

andrewc-arm commented Jan 23, 2020

mergify bot commented Jan 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewc-arm commented Dec 16, 2019 •

edited

Loading

andrewc-arm commented Dec 17, 2019 •

edited

Loading

andrewc-arm commented Jan 22, 2020 •

edited

Loading