-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add support for RTC status/read/write commands #152
Conversation
Include EEPROM & RTC detection in ctest example
My RTC emulation may be useful for implementing the write RTC feature. It's been some time since I experimented with this. But the procedure should be as follows:
|
I've tested the ROM and it's hanging at "Waiting for RTC to stop...". (using the UltraPIF RTC emulation.) I've noticed that the control register seems to be in the wrong byte order. I'm receiving 0x0400 instead of 0x0004. I think the problem is here: Line 189 in 867d7ae
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is spectacular work, and the fact that you also allowed progresses to be made in emulators, everdrive, and even ultrapif is simply outstanding!
I've made a first review from the phone. The code seems ok apart from a potential alignment bug, so I focused on documentation and API ergonomic.
src/rtc.c
Outdated
* 0x0100 is the block 1 write-protection bit. | ||
* 0x0200 is the block 2 write-protection bit. | ||
* | ||
* RTC block 1 has an unknown purpose and is typically unimplemented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unimplemented by whom? Emulators? I'd mention it explicitly.
Do we know the actual RTC chip used by animal crossing? Its data sheet might provide some insights even di the communication is shielded by the PIF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not found any documentation about what block 1 is for. No emulator implements it (that I have found), the UltraPIF and 64drive also do not implement it.
As far as I can tell, Animal Forest does not ever access block 1, so its purpose is a complete mystery. I'll do a bit more research and see if I can figure out what it is from the actual component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly, block 1 is an 8 byte battery backed SRAM.
I could do some tests if needed. I have a real RTC and with the UltraPIF and a debugger I can directly send commands to the RTC and see the results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/rtc.c
Outdated
* @brief Joybus real-time clock interface. | ||
* | ||
* The Joybus real-time clock is an accessory that was only included on | ||
* one official cartridge that was only available in Japan. The Joybus RTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, it was an official option supported by Nintendo, implemented in PIF firmware, but only animal crossing decided to use it in its cartridge. I'd specify this, because otherwise it sounds like a game-specific hardware extension which would have very little relevance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also specify here in this first paragraph that it's supported by some development kits (64drive / ed64) and some emulators (list here which)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jago85 is the RTC actually a feature that was always intended to be supported by the PIF, or is it a game-specific convention that's mostly implemented on the cartridge circuitry? From what I can tell, it behaves a lot like EEPROM (RTC is on the same PIF channel as EEPROM) but responds to different command identifiers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I really don't know when they were planning this. Your question is about the original PIF, isn't it?
Yes, the RTC is completely implemented inside the cart, just using the SI to transport the data. The PIF doesn't care what is connected to the bus and what the commands are good for. It just shifts the data out to and in from one of the five SI channels. So from the PIF's point of view, the interface is pretty generic.
I've heared that Animal Forest was originally planned for the 64DD. As it flopped they ported the game to cartridge and needed a replacement RTC. So they designed an RTC chip with SI interface. And it was designed with the ability to coexist with an EEPROM chip on the same bus.
@rasky This is ready for review again. I have integrated the RTC Subsystem with newlib's I have also refactored the RTC Subsystem in anticipation for supporting the 64DD RTC (cen64 has a partial implementation of it that can read time, so we have a starting point). Conceivably, we could also support a "Simulated RTC" that would have to be set on startup and maintain the date/time as long as the system is powered on. This would be "better than nothing" for players who want to play RTC-enabled homebrew but don't actually have RTC-capable hardware. The I considered implementing your suggestion of only reading the RTC once during initialization, but it got complicated. If we do end up implementing the "Simulated RTC", we will basically end up with what you proposed though. I need to put together some timing tests to figure out what the real cost of calling each of these API methods is -- unfortunately I don't have an RTC on my flash cart, so I'll have to enlist some EverDrive and 64drive owners in Discord to get me hard numbers. Of course, this will all be better once we integrate the Joybus commands with your fancy new kernel. 😄 Please let me know if you have any other suggestions here. Thank you for your feedback! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have integrated the RTC Subsystem with newlib's gettimeofday and added a demo using ISO C time functions to the ctest example.
Great thanks!
Conceivably, we could also support a "Simulated RTC" that would have to be set on startup and maintain the date/time as long as the system is powered on. This would be "better than nothing" for players who want to play RTC-enabled homebrew but don't actually have RTC-capable hardware.
One option I thought of is that we could add a feature to the debugging library to read the current time via USB (extending the UNFLoader protocol, so to be able to read the time on the host PC). But given that both 64drive hw2 and Everdrive X7 have a RTC already, it looks like this feature would be only useful to 64drive hw1 owners. Not sure it's worth it.
The rtc_get function now has a cache that invalidates after 1 second so that homebrew can read the clock every frame without paying the cost of hitting the SI just to see if the clock ticked.
I think the cache is a reasonable compromise. What I suggested would still be superior because it would basically means that rtc_get / time(NULL)
is always ~free, but I can see that it's going to be more complicated.
Of course, this will all be better once we integrate the Joybus commands with your fancy new kernel. 😄
Sure! Joybus is pretty slow in general so doing it in background sounds like a good idea anyway.
@@ -66,8 +66,10 @@ install: libdragon.a libdragonsys.a | |||
install -m 0644 include/debug.h $(INSTALLDIR)/mips64-elf/include/debug.h | |||
install -m 0644 include/usb.h $(INSTALLDIR)/mips64-elf/include/usb.h | |||
install -m 0644 include/console.h $(INSTALLDIR)/mips64-elf/include/console.h | |||
install -m 0644 include/joybus.h $(INSTALLDIR)/mips64-elf/include/joybus.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we hold on installing this (and making joybus_exec
public API)? I think the API might change as part of the integration with the kernel, so if we can hold up a little on this, we can avoid later breaking things that start depending on it.
We can keep the include file internal, so that only libdragon itself can use joybus_exec
for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on the fence about this. On the one hand, I agree that it's probably a good idea not to expose APIs that are subject to change in the near future, but I also see immense utility in exposing the Joybus commands outside of libdragon (for example, this Randnet Keyboard test ROM: meeq@44aff08)
I think there's going to be a huge shake-up in many of the APIs once the kernel lands, so I think we need to have a clear plan for the post-kernel transition process that either minimizes breakage or draws hard lines on where breakage is okay.
I've tested the new ROM again and can confirm that it works with the real RTC. It's working with the emulation of the UltraPIF and the Brutzelkarte too. Good job! 😄 |
After a chat with @meeq and his latest rework for using the 64-bit timer for cache invalidation, this PR is ready to go in for me. @anacierdem @DragonMinded the only minor point left here is whether we want to expose |
Im ok to have it exposed with a comment to say it maybe subject to change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things. Will merge when you say ok
examples/rtctest/Makefile
Outdated
|
||
# Byte-swap ROM | ||
ifeq ($(N64_BYTE_SWAP),true) | ||
ROM_EXTENSION = .v64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can change these to match the new version as well now
month = month + 12; | ||
year = year - 1; | ||
} | ||
rtc_time->week_day = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide a reference for this I could not find this exact method 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s based on a snippet that @networkfusion sent to me — he said it was from a homework problem for one of his Computer Science classes 😄
I’ve verified that it’s correct, at least for the date ranges that are supported by RTC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, written by me when in college as part of a paper to check if a calendar algorithm was incorrect and prove why. (I will see if I can dig out the original source!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attribution would be welcome 😉
WPFOdrinalCalculator.zip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(BTW, my teacher thought that there was nothing wrong with the original algorithm until he saw my paper 🤣 )
examples/rtctest/Makefile
Outdated
N64TOOL = $(SDK_DIR)/bin/n64tool | ||
|
||
# LibDragon Flags | ||
OUT_SIZE = 1052672B # 52672B HEADER + 1M (minimum) ROM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This explanation is incorrect. Rather the minimum size is 1 MiB = 1048576 bytes (not decimal megabytes) and the header size is 4096 bytes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’re right, that was a gnarly workaround for a bug in n64tool that has now been fixed. Good catch.
Changed - Improve fastpath of dfs_read (DragonMinded/libdragon#133) - Refactor n64tool (DragonMinded/libdragon#153, DragonMinded/libdragon#155) Fixed - Zero-initialize the token array to avoid -Werror=maybe-uninitialized (DragonMinded/libdragon#134) - Initialize arguments to main libdragon entrypoint (DragonMinded/libdragon#136) - SD support fixes and dragonfs fopen fix (DragonMinded/libdragon#137) - lib/include paths in tests Makefile (DragonMinded/libdragon#138) - Reenable test_timer_ticks for emulators (DragonMinded/libdragon#140) - n64tool: return error in case the seek offset required backward seek (DragonMinded/libdragon#144) - Add missing extern "C" in debug.h (DragonMinded/libdragon#146) - Ensure C++ global constructors are not garbage collected by ld (DragonMinded/libdragon#148) - Fix clipped RDP rectangle drawing (DragonMinded/libdragon#147) Added - restart_timer and new_timer_stopped functions (DragonMinded/libdragon#131) - dfs_rom_addr (DragonMinded/libdragon#133) - Implement EEPROM Filesystem and test ROM (DragonMinded/libdragon#125) - ed64romconfig binary (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - Support for RTC status/read/write commands (DragonMinded/libdragon#152)
Changed - Improve fastpath of dfs_read (DragonMinded/libdragon#133) - Refactor n64tool (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - It no longer support byte-swapping and only generates a z64 file. Fixed - Zero-initialize the token array to avoid -Werror=maybe-uninitialized (DragonMinded/libdragon#134) - Initialize arguments to main libdragon entrypoint (DragonMinded/libdragon#136) - SD support fixes and dragonfs fopen fix (DragonMinded/libdragon#137) - lib/include paths in tests Makefile (DragonMinded/libdragon#138) - Reenable test_timer_ticks for emulators (DragonMinded/libdragon#140) - n64tool: return error in case the seek offset required backward seek (DragonMinded/libdragon#144) - Add missing extern "C" in debug.h (DragonMinded/libdragon#146) - Ensure C++ global constructors are not garbage collected by ld (DragonMinded/libdragon#148) - Fix clipped RDP rectangle drawing (DragonMinded/libdragon#147) Added - restart_timer and new_timer_stopped functions (DragonMinded/libdragon#131) - dfs_rom_addr (DragonMinded/libdragon#133) - Implement EEPROM Filesystem and test ROM (DragonMinded/libdragon#125) - ed64romconfig binary (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - Support for RTC status/read/write commands (DragonMinded/libdragon#152)
Changed - Removed make, download, init, buildDragon, prepareDragon, and installDependencies NPM scripts - Update the necessary vscode and travis configuration - Update the readme to match - this also fixes #31 - Improve fastpath of dfs_read (DragonMinded/libdragon#133) - Refactor n64tool (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - It no longer support byte-swapping and only generates a z64 file. - Change test bench Makefile to reflect latest changes Fixed - Zero-initialize the token array to avoid -Werror=maybe-uninitialized (DragonMinded/libdragon#134) - Initialize arguments to main libdragon entrypoint (DragonMinded/libdragon#136) - SD support fixes and dragonfs fopen fix (DragonMinded/libdragon#137) - lib/include paths in tests Makefile (DragonMinded/libdragon#138) - Reenable test_timer_ticks for emulators (DragonMinded/libdragon#140) - n64tool: return error in case the seek offset required backward seek (DragonMinded/libdragon#144) - Add missing extern "C" in debug.h (DragonMinded/libdragon#146) - Ensure C++ global constructors are not garbage collected by ld (DragonMinded/libdragon#148) - Fix clipped RDP rectangle drawing (DragonMinded/libdragon#147) - Enable byte swap flag for the make action and update documentation accordingly - Skip the second parameter to the libdragon command as well - Enable --byte-swap flag for the make action Added - restart_timer and new_timer_stopped functions (DragonMinded/libdragon#131) - dfs_rom_addr (DragonMinded/libdragon#133) - Implement EEPROM Filesystem and test ROM (DragonMinded/libdragon#125) - ed64romconfig binary (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - Support for RTC status/read/write commands (DragonMinded/libdragon#152) - Generic libdragon NPM script
Changed - Removed make, download, init, buildDragon, prepareDragon, and installDependencies NPM scripts - Update the necessary vscode and travis configuration - Update the readme to match - this also fixes #31 - Improve fastpath of dfs_read (DragonMinded/libdragon#133) - Refactor n64tool (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - It no longer support byte-swapping and only generates a z64 file. - Change test bench Makefile to reflect latest changes Fixed - Zero-initialize the token array to avoid -Werror=maybe-uninitialized (DragonMinded/libdragon#134) - Initialize arguments to main libdragon entrypoint (DragonMinded/libdragon#136) - SD support fixes and dragonfs fopen fix (DragonMinded/libdragon#137) - lib/include paths in tests Makefile (DragonMinded/libdragon#138) - Reenable test_timer_ticks for emulators (DragonMinded/libdragon#140) - n64tool: return error in case the seek offset required backward seek (DragonMinded/libdragon#144) - Add missing extern "C" in debug.h (DragonMinded/libdragon#146) - Ensure C++ global constructors are not garbage collected by ld (DragonMinded/libdragon#148) - Fix clipped RDP rectangle drawing (DragonMinded/libdragon#147) - Enable byte swap flag for the make action and update documentation accordingly - Skip the second parameter to the libdragon command as well - Enable --byte-swap flag for the make action Added - restart_timer and new_timer_stopped functions (DragonMinded/libdragon#131) - dfs_rom_addr (DragonMinded/libdragon#133) - Implement EEPROM Filesystem and test ROM (DragonMinded/libdragon#125) - ed64romconfig binary (DragonMinded/libdragon#153, DragonMinded/libdragon#155) - Support for RTC status/read/write commands (DragonMinded/libdragon#152) - Generic libdragon NPM script
The Joybus commands to read/write to RTC are based off of the UltraPIF implementation (thanks @jago85!) and 64drive (thanks marshallh!)
Calling
rtc_init
will detect if an RTC is available, and if so will enable it and hook into newlib'sgettimeofday
. From there, software can either callrtc_get
to read the current time into anrtc_time_t
struct or use the ISO C time functions such astime
. To adjust the current time, first callrtc_is_writable
to determine if RTC writes are supported, and if so callrtc_set
with an updatedrtc_time_t
struct.Introduces a new
rtctest
example that demonstrates reading and writing the date/time from the cartridge RTC:Compiled
rtctest
ROM:rtctest.zip
This ROM has been successfully tested on 64drive HW2 🎉
This ROM does not allow changing the date/time on EverDrive64 X7, since the firmware does not support the RTC write SI command.
This ROM also does not allow setting the date/time on any emulator I have tested, since none of them handle the RTC write SI command. (Except my branch of cen64 that adds support for RTC writes: n64dev/cen64#205)