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

Fixes from n64 branch #7

Open
wants to merge 2 commits into
base: embedded
Choose a base branch
from

Conversation

clktmr
Copy link

@clktmr clktmr commented Jun 12, 2024

Here are some changes from #6 that are unrelated to mips64 support. I created a separate PR to get your opinion on this. Please have a look at the commits.

This mainly prevented to print panics before the systimer was set up.
See the usleep() calls in freezetheworld().

This comes with the possibility to have a non-monotonic jump in time
when the systimer is finally set.  Systimer implementations must respect
the dummyNanoseconds if there is a risk.
@@ -75,7 +75,9 @@ import (
// Tasker code does not use FPU so the architecture specific context switch
// code can avoid saving/restoring FPU context if not need.

func dummyNanotime() int64 { return 1 }
var dummyNanoseconds int64
Copy link
Author

Choose a reason for hiding this comment

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

Should be an atomic to do this properly

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice and simple solution.

Yes, use internal/atomic.Xadd64. It should work on a 64-bit CPU without any problem but I don't know what about 32-bit ARM.

Try test it on your Teensy if you can. Or apply to the the wip and try to use GOTARGET=noostest GOARCH=thumb emgo test. You must set the system timer back to this implementation because the ARMv7-M SYSTICK is used by default. Don't forgot to set the start value of dummyNanoseconds to something bigger that the last rtos.Nanotime().

For now I plan to collect such new noos tests here: https://github.com/embeddedgo/noostest/tree/master/tests

If you are unsure how to do all this please ask me. I can also try to invent a proper test myself if yow want.

@michalderkacz
Copy link
Collaborator

michalderkacz commented Jul 5, 2024

I cherry-picked 4b8a81f into master-embedded.

According dummyNanotime I checked all usleep and usleep_no_g calls:

runtime/proc.go:1014.3-1014.8:    usleep(1000)
runtime/proc.go:1029.3-1029.8:    usleep(1000)
runtime/proc.go:1032.2-1032.7:    usleep(1000)
runtime/proc.go:1034.2-1034.7:    usleep(1000)
runtime/proc.go:5973.3-5973.8:    usleep(delay)
runtime/proc.go:6841.8-6841.13:  usleep(3)

runtime/proc.go:576.2-576.12:      usleep_no_g(100)
runtime/proc.go:2547.4-2547.14:  usleep_no_g(1)

I think we can try to implement both usleep and usleep_no_g as no-op function (like in os_js.go).

As I see, the usleep is used mainly to give the another threads a chance to run before something is done, but it's always a "best effort" action. So maybe we can implement usleep like this:

func usleep(us uint32) {
  for i := range (1 + us/16) {
    osyield()
    // or maybe yield_m()
  }
}

@michalderkacz
Copy link
Collaborator

It seems that sysmon uses usleep as some kind of real timer. So I propose to implement the usleep and usleep_no_g functions this way:

func usleep(us uint32) {
  if thetasker.nanotime != dummyNanotime && tasker.newnanotime == nil {
    nanosleep(uint64(us) * 1000)
    return
  }
  for i := range (1 + us/32) {
    osyield()
  }
}

The syssetsystim1 should set tasker.newnanotime to nil after setting tasker.nanotime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants