S5JS100 target doesn't compile with blinky-baremetal

[MarceloSalazar]

Description of defect

S5JS100 target fails to compile blinky-baremetal in Mbed OS 6

GCC Error

[Fatal Error] modem_io_device.h@24,10: pbuf.h: No such file or directory
[ERROR] In file included from ./mbed-os/targets/TARGET_Samsung/TARGET_SIDK_S5JS100/dcxo_update.cpp:20:
./mbed-os/targets/TARGET_Samsung/TARGET_SIDK_S5JS100/modem/modem_io_device.h:24:10: fatal error: pbuf.h: No such file or directory
   24 | #include "pbuf.h"

Arm error

[Fatal Error] s5js100_systemreset.c@51,10: 'rtx_os.h' file not found
[ERROR] ./mbed-os/targets/TARGET_Samsung/TARGET_SIDK_S5JS100/device/s5js100_systemreset.c:51:10: fatal error: 'rtx_os.h' file not found
#include "rtx_os.h"

Target(s) affected by this defect ?

S5JS100

Toolchain(s) (name and version) displaying this defect ?

Arm Compiler 6 and GCC 9

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.0.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Mbed CLI 1.10.4

How is this defect reproduced ?

mbed import https://github.com/ARMmbed/mbed-os-example-blinky-baremetal
cd mbed-os-example-blinky
mbed compile -t <toolchain> -m <target>

@armmbed/team-samsung

[ciarmcom]

@MarceloSalazar thank you for raising this issue.Please take a look at the following comments:

We cannot automatically identify a release based on the version of Mbed OS that you have provided.
Please provide either a single valid sha of the form #abcde12 or #3b8265d70af32261311a06e423ca33434d8d80de
or a single valid release tag of the form mbed-os-x.y.z .
E.g. '(d0d35c2)' has not been matched as a valid tag or sha.
NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered.
Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2710

[0xc0170]

rtx_os.h should not be included by a target, that should rely on rtos header rather . A question is why s5js100_systemreset implementation needs it..

[LDong-Arm]

rtx_os.h should not be included by a target, that should rely on rtos header rather . A question is why s5js100_systemreset implementation needs it..

Apparently it doesn't need it. Same for pbuf.h. They might be leftover code I guess.

After removing those redundant includes, we hit other issues in TARGET_SIDK_S5JS100, mainly

• Source files include mbed_trace.h even when tracing is not enabled (which is the default).
• Vendor-specific features (e.g. ModemIoDevice) that use RTOS don't check for MBED_CONF_RTOS_PRESENT. We can safely add the checks as they are not used by Mbed OS.

[LDong-Arm]

Okay, this target's hal_sleep() and hal_deepsleep() implementations require RTOS. The power management uses APIs like Thread and Semaphore.

hal_sleep() uses an RTOS idle hook:

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);
    initialize_policy = 1;
}
...
void hal_sleep(void)
{
    if (initialize_policy == 0) {
        set_sleep_policy();
    }

@MarceloSalazar @0xc0170 Is bare metal support a requirement for all targets? And I don't see the advantage of s5js100_idle_hook over the Mbed OS default idle hook:

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();
	}

[LDong-Arm]

I raised a question in the original PR that added the target https://github.com/ARMmbed/mbed-os/pull/12106/files#r441556994

[MarceloSalazar]

@LDong-Arm thanks for looking into this

Is bare metal support a requirement for all targets?

Not at this time. We'd like to understand the reason behind the error and check with @ARMmbed/team-samsung the possibility to fix this it at some point soon.

In the meantime, we should consider adding an enhancement to the tools to inform that a target doesn't support baremetal or the full profile (FYI @rwalton-arm )

[LDong-Arm]

#13265 created