add target port: curie_bsp #1234

jiangzidong · 2016-08-02T09:32:58Z

Intel® Curie BSP is the SDK that will help you developing software on Curie based boards, for example with the Arduino 101 board (AKA Genuino 101). https://github.com/CurieBSP/main/releases

We plan to add the curie_bsp port in the jerryscript/targets, so that CurieBSP users can develop js app.

jiangzidong · 2016-08-02T09:46:04Z

we will only compile files in jerry-core, jerry-libm and targets/curie_bsp
3 folders in the targets/curie_bsp:

source is the platform(curie) related code, such like date process, setjmp iamcu assemly implementation and io implementation.
include contains the header file needed by source
jerry-app is a curie_bsp project folder, which includes the main.c, project-config-file and memory-config-file

jiangzidong · 2016-08-02T09:48:46Z

setup.py is used to create the makefile and jerry-config.

Because of the flash limitation of arduino101, we build the minimal_cp profile (with ERROR_BUILTIN enabled), and the mem_heap size is 10k

jiangzidong · 2016-08-05T06:44:11Z

@LaszloLango Hi, do you have any comments about this patch?

LaszloLango · 2016-08-05T06:51:18Z

@jiangzidong, sorry I had no time to check the full patch. It is quite big :) AFAIK, @galpeter also started to check it. I'll review it today and give you some feedback.

jiangzidong · 2016-08-05T07:00:01Z

@LaszloLango Thanks. I'm just afraid that the patch is neglected :)
Of course you should take your time.

LaszloLango · 2016-08-05T08:30:51Z

targets/curie_bsp/include/alloca.h

+
+#define alloca(size) __builtin_alloca(size)
+
+#endif


missing new line at the end of file

(style) #endif not followed by a /* !ALLOCA_H */ comment (same in inttypes.h)

galpeter · 2016-08-05T08:38:25Z

targets/curie_bsp/setup.py

+  file_to_be_created.close()
+
+def main(curie_path, jerry_path):
+  pass


why do we need the pass here?

jiangzidong · 2016-08-08T08:09:50Z

Hi, @galpeter @LaszloLango I fixed according to the comments above, except the " keep those dirs clean". Because kbuild.mks in each source folder are requried by CurieBSP. https://github.com/CurieBSP/main/blob/master/doc/build_kbuild.md

Do you have any further comments? Thanks

zherczeg · 2016-08-11T05:50:26Z

Hi @jiangzidong both Laszlo and Peter are not available this and next week. Can you wait a bit for review?

jiangzidong · 2016-08-11T05:53:11Z

@zherczeg, of course~ Thanks for your reply :)

akosthekiss · 2016-08-11T13:59:26Z

While the others are away, let me give some feedback:

Why are include/alloca.h and include/inttypes.hneeded? I cannot see them included directly from the now-added files. Or are they transitively included from include/jerry-libc-defs.h somehow?
Why are rand(), srand(), setjmp(), and longjmp() reimplemented? You don't build jerry-libc, which means, I guess, that you rely on some system libc implementation. Does it not implement them?
I don't think that you need to implement jerry_port_default_get_log_level, jerry_port_default_set_log_level, jerry_port_default_set_abort_on_fail, and jerry_port_default_is_abort_on_fail for the curie port. As their name suggests, these functions are specific to the default port. The currie port (or, at least, the main app you've provided, does not use such functions. You can implement the jerry_port_log and jerry_port_fatal port functions without them. Also note that jerry_port_fatal does not even have to call either exit or abort. It's your port implementation, so it's your call. You might simply implement it as jerry_port_log(JERRY_LOG_LEVEL_ERROR, "fatal!!!"); while (true) {} without any further calls.
JerryScript code base is under Apache 2.0 license, but some of the proposed files are under what seems to me as the 3-clause BSD. I'm not a lawyer, but I'm not sure we want to mix different licenses. What was your motivation for using a different license from the rest of the code?

More comments will follow inlined.

akosthekiss · 2016-08-11T14:02:27Z

targets/curie_bsp/include/jerry-libc-defs.h

+#define __attr_unused___   __attribute__((unused))
+#define __attr_used___     __attribute__((used))
+#define __attr_noreturn___ __attribute__((noreturn))
+#define __attr_noinline___ __attribute__((noinline))


This define/attribute is not used anywhere, so should be removed. If exit and abort will be removed as suggested in the general comment, all the other defines can go away, too.

galpeter · 2016-08-25T18:43:09Z

targets/curie_bsp/source/curie-bsp-port.c

+    va_list args;
+    va_start (args, format);
+    length = vsnprintf (buf, 256, format, args);
+    buf[length]='\0';


Missing space around =.

jiangzidong · 2016-08-26T02:19:06Z

@galpeter Thanks for your comments!
1), No, we don't need the x permission of the source files. It should be my mistake. Already fixed
2), Wow, great! I didn't notice the usage of OUT_SRC before. I suggest it should be a new
commit/PR, because you did a lot of work. what do you think about it?

akosthekiss · 2016-08-26T07:25:04Z

+1 to follow-up patch with OUT_SRC

zherczeg · 2016-08-26T08:48:42Z

targets/curie_bsp/jerry_app/quark/main.c

+}
+
+
+void help ()


Only one newline.

galpeter · 2016-08-26T09:01:12Z

@jiangzidong adding it as a follow-up patch works for me.

zherczeg · 2016-08-26T09:45:26Z

Ok, so shall we land this?

jiangzidong · 2016-08-26T13:58:13Z

@zherczeg, updated according to your comments. Thanks.

LaszloLango · 2016-08-26T14:02:57Z

still LGTM

zherczeg · 2016-08-26T16:50:45Z

LGTM. Is it in the latest master?

jiangzidong · 2016-08-27T10:45:27Z

@zherczeg, just rebased to the latest master

JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

zherczeg · 2016-08-27T12:12:17Z

Do you want this patch be part of the 1.0 release?

jiangzidong · 2016-08-27T13:39:55Z

Yes. It will be great to be part of 1.0 release, so that Jerry users (who download the v1.0) will know how to port into CurieBSP and run in Arduino101.

zherczeg · 2016-08-28T04:48:57Z

Added. Thank you.

tilmannOSG · 2016-08-29T08:01:55Z

@zherczeg Can you please cherry-pick this commit to the 1.0 release branch?
Thanks!

tilmannOSG · 2016-08-29T08:04:31Z

@zherczeg Sorry for the noise, I just realized that the change has already been cherry-picked to the release branch :)

LaszloLango added development Feature implementation jerry-port Related to the port API or the default port implementation labels Aug 2, 2016

jiangzidong force-pushed the curie_bsp branch from d7f26c8 to 7217220 Compare August 3, 2016 01:59

LaszloLango reviewed Aug 5, 2016
View reviewed changes

jiangzidong force-pushed the curie_bsp branch from 7217220 to c7d716f Compare August 5, 2016 08:38

galpeter reviewed Aug 5, 2016
View reviewed changes

targets/curie_bsp/setup.py

file_to_be_created.close()

def main(curie_path, jerry_path):

pass

Copy link

Contributor

galpeter Aug 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the pass here?

jiangzidong force-pushed the curie_bsp branch from c7d716f to d8ba329 Compare August 5, 2016 13:27

akosthekiss reviewed Aug 11, 2016
View reviewed changes

galpeter reviewed Aug 25, 2016
View reviewed changes

jiangzidong force-pushed the curie_bsp branch 2 times, most recently from b4c1045 to bb15f18 Compare August 26, 2016 02:08

zherczeg reviewed Aug 26, 2016
View reviewed changes

targets/curie_bsp/jerry_app/quark/main.c

}

void help ()

Copy link

Member

zherczeg Aug 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one newline.

jiangzidong force-pushed the curie_bsp branch from bb15f18 to 0838b7e Compare August 26, 2016 13:56

jiangzidong force-pushed the curie_bsp branch from 0838b7e to 04e597a Compare August 27, 2016 10:44

add target port: curie_bsp

04e597a

JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

zherczeg merged commit 04e597a into jerryscript-project:master Aug 27, 2016

zherczeg mentioned this pull request Jan 19, 2017

Nominating Zidong Jiang (@jiangzidong) to become a JerryScript committer #1535

Closed

add target port: curie_bsp #1234

add target port: curie_bsp #1234

Uh oh!

Conversation

jiangzidong commented Aug 2, 2016

Uh oh!

jiangzidong commented Aug 2, 2016

Uh oh!

jiangzidong commented Aug 2, 2016

Uh oh!

jiangzidong commented Aug 5, 2016

Uh oh!

LaszloLango commented Aug 5, 2016

Uh oh!

jiangzidong commented Aug 5, 2016

Uh oh!

LaszloLango Aug 5, 2016

Choose a reason for hiding this comment

Uh oh!

akosthekiss Aug 12, 2016

Choose a reason for hiding this comment

Uh oh!

galpeter Aug 5, 2016

Choose a reason for hiding this comment

Uh oh!

jiangzidong commented Aug 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zherczeg commented Aug 11, 2016

Uh oh!

jiangzidong commented Aug 11, 2016

Uh oh!

akosthekiss commented Aug 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akosthekiss Aug 11, 2016

Choose a reason for hiding this comment

Uh oh!

galpeter Aug 25, 2016

Choose a reason for hiding this comment

Uh oh!

jiangzidong commented Aug 26, 2016

Uh oh!

akosthekiss commented Aug 26, 2016

Uh oh!

zherczeg Aug 26, 2016

Choose a reason for hiding this comment

Uh oh!

galpeter commented Aug 26, 2016

Uh oh!

zherczeg commented Aug 26, 2016

Uh oh!

jiangzidong commented Aug 26, 2016

Uh oh!

LaszloLango commented Aug 26, 2016

Uh oh!

zherczeg commented Aug 26, 2016

Uh oh!

jiangzidong commented Aug 27, 2016

Uh oh!

zherczeg commented Aug 27, 2016

Uh oh!

jiangzidong commented Aug 27, 2016

Uh oh!

zherczeg commented Aug 28, 2016

Uh oh!

tilmannOSG commented Aug 29, 2016

Uh oh!

tilmannOSG commented Aug 29, 2016

Uh oh!

Uh oh!

jiangzidong commented Aug 8, 2016 •

edited

Loading

akosthekiss commented Aug 11, 2016 •

edited

Loading