OpenBIOS: Add ZN-1 support (and lay the groundwork for ZN-2 support)#1987
Open
spicyjpeg wants to merge 3 commits intogrumpycoders:mainfrom
Open
OpenBIOS: Add ZN-1 support (and lay the groundwork for ZN-2 support)#1987spicyjpeg wants to merge 3 commits intogrumpycoders:mainfrom
spicyjpeg wants to merge 3 commits intogrumpycoders:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds OpenBIOS support for the ZN arcade board variant, including hardware register definitions, a dedicated MIPS boot routine, board-specific kernel functions for system information retrieval, and build system modifications for conditional ZN/system573 compilation. Changes
Sequence DiagramssequenceDiagram
participant Boot as Boot Routine<br/>(zn.s)
participant HW as Hardware<br/>(Registers)
participant RAM as Memory Controller<br/>(DRAM Config)
participant Kernel as Kernel<br/>(misc.c)
participant Main as Main Init<br/>(main.c)
Boot->>HW: Write PSX bus config
Boot->>RAM: Probe ZN_RAM_CONFIGS
Boot->>RAM: Read board revision
Boot->>RAM: Set DRAM controller
Boot->>Boot: Clear registers & cop0/cop2
Boot->>Boot: Copy data segment
Boot->>Boot: Initialize BSS
Boot->>Kernel: Call _ucsdk_start()
Kernel->>HW: Query ZN_BOARD_CONFIG
Kernel->>Kernel: Store __globals60.ramsize
Main->>Main: clearZNRegisters()
Main->>Main: printBoardConfiguration()
Main->>HW: Write 0x0A to 0x1f802080
Main->>Main: Enter stop loop
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/mips/openbios/kernel/misc.c (2)
68-96: Missingcase 8andcase 10/case 11— intentional gaps?The switch jumps from case 7 (VRAM size) to case 9 (SPU RAM size), and from case 9 to case 12/13. If this mirrors the original Sony kernel's behavior, a brief comment noting these indices are intentionally undefined (return 0) would help future readers.
The static analysis flagged cyclomatic complexity of 14 (threshold 9), but this is a flat lookup table — restructuring wouldn't improve clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mips/openbios/kernel/misc.c` around lines 68 - 96, The getSystemInfo switch omits case 8 and cases 10/11 which look like intentional undefined indices; update the getSystemInfo function to document this by adding a concise comment above the switch (or inline near the gaps) stating that indices 8, 10 and 11 are intentionally undefined and return 0 to mirror the original Sony kernel behavior, and optionally add explicit case labels (case 8: case 10: case 11:) that return 0 to make the intent clear; reference the getSystemInfo function and related symbols like s_kernelFlags, s_unknownFlag, getVRAMSize and getSPURAMSize when locating where to add the comment or explicit cases.
42-50: ZN-2:s_kernelFlagsis hardcoded to the ZN-1 value.Line 44 hardcodes
s_kernelFlags = 0x11001(ZN-1) while the comment notes ZN-2 should be0x13000. Since the board config register is already read at runtime for revision/VRAM/SPU, consider derivings_kernelFlagsfromZN_BOARD_CONFIGas well (e.g., checking the board revision) so this doesn't become a latent bug when ZN-2 testing begins.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mips/openbios/kernel/misc.c` around lines 42 - 50, s_kernelFlags is hardcoded to the ZN-1 value (0x11001) but should reflect ZN-2 (0x13000) when the board revision indicates ZN-2; change s_kernelFlags to be derived at runtime (either compute it in an init path or replace the const with a getter) using the existing ZN_BOARD_CONFIG parsing (reuse getBoardRevision() or the same ((ZN_BOARD_CONFIG >> 5) & 7) - 2 expression) and return 0x11001 for ZN-1 and 0x13000 for ZN-2 so all consumers of s_kernelFlags observe the correct value for the detected board.src/mips/openbios/main/main.c (1)
298-311: Intentional "Borad" typo and original-BIOS-faithful output — looks good.The comment on lines 298–301 makes the intent clear. One small note:
revision - 2on line 308 can produce negative values when bits [7:5] ofZN_BOARD_CONFIGare 0 or 1. If this matches the original BIOS behavior, it's fine, but worth a brief inline comment to prevent a future contributor from "fixing" it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mips/openbios/main/main.c` around lines 298 - 311, The calculation "revision - 2" in printBoardConfiguration can produce negative values for low ZN_BOARD_CONFIG revision bits; add a brief inline comment next to the expression (or near the computation of revision) stating that the subtraction is intentional to match original BIOS behavior and may yield negative values so future contributors do not "fix" it; reference the revision variable, the subtraction (revision - 2), ZN_BOARD_CONFIG and the psxprintf call so the comment is clearly colocated with the logic in printBoardConfiguration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/mips/openbios/kernel/misc.c`:
- Around line 68-96: The getSystemInfo switch omits case 8 and cases 10/11 which
look like intentional undefined indices; update the getSystemInfo function to
document this by adding a concise comment above the switch (or inline near the
gaps) stating that indices 8, 10 and 11 are intentionally undefined and return 0
to mirror the original Sony kernel behavior, and optionally add explicit case
labels (case 8: case 10: case 11:) that return 0 to make the intent clear;
reference the getSystemInfo function and related symbols like s_kernelFlags,
s_unknownFlag, getVRAMSize and getSPURAMSize when locating where to add the
comment or explicit cases.
- Around line 42-50: s_kernelFlags is hardcoded to the ZN-1 value (0x11001) but
should reflect ZN-2 (0x13000) when the board revision indicates ZN-2; change
s_kernelFlags to be derived at runtime (either compute it in an init path or
replace the const with a getter) using the existing ZN_BOARD_CONFIG parsing
(reuse getBoardRevision() or the same ((ZN_BOARD_CONFIG >> 5) & 7) - 2
expression) and return 0x11001 for ZN-1 and 0x13000 for ZN-2 so all consumers of
s_kernelFlags observe the correct value for the detected board.
In `@src/mips/openbios/main/main.c`:
- Around line 298-311: The calculation "revision - 2" in printBoardConfiguration
can produce negative values for low ZN_BOARD_CONFIG revision bits; add a brief
inline comment next to the expression (or near the computation of revision)
stating that the subtraction is intentional to match original BIOS behavior and
may yield negative values so future contributors do not "fix" it; reference the
revision variable, the subtraction (revision - 2), ZN_BOARD_CONFIG and the
psxprintf call so the comment is clearly colocated with the logic in
printBoardConfiguration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds preliminary support for Sony's ZN-1 arcade PCB to OpenBIOS. Support is currently limited to booting an executable embedded in the ROM; the game-specific security and program ROM bootstrap code present in Sony's ZN BIOSes has not been reimplemented.
As with the System 573 port, the bulk of the changes is in the entry point (
boot/zn.s), which has been modified with the appropriate bus configuration values as well as some additional logic to correctly set up main RAM depending on the board revision. ThesetMemSize()API call has been expanded to support 4 and 16 MB RAM, reflecting the implementation found in the ZN and System 573 kernels. The previously missinggetSystemInfo()function has been added as well, returning the correct main RAM, VRAM and SPU RAM sizes for all currently supported boards.The build has so far been tested and confirmed to work on a ZN-1 board but not on a ZN-2, for which more investigation is required. The
getSystemInfo()implementation has not been tested but it is trivial enough that there should be no bugs in it (no software is currently known to use it anyway).