Jtag single bit #48
Jtag single bit #48
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request updates several modules to enforce a consistent formatting style. Function declarations are modified to place opening curly braces on a new line. A new lambda, Changes
Sequence Diagram(s)sequenceDiagram
participant JSON as JSON Input
participant Parser as parse_settings
participant Lambda as get_value_bool
participant Settings as HSLink_Setting
JSON->>Parser: Provide JSON string
Parser->>Lambda: Check for "jtag_single_bit_mode"
Lambda-->>Parser: Return boolean value
Parser->>Settings: Set jtag_single_bit_mode
sequenceDiagram
participant JTAG as SPI_PORT_JTAG_SETUP
participant Setting as HSLink_Setting
participant GPIO as GPIO Configurator
JTAG->>Setting: Check jtag_single_bit_mode
alt Single Bit Mode Enabled
JTAG->>GPIO: Configure TCK, TDI, TDO for alternate function
else Standard Mode
JTAG->>GPIO: Apply standard configuration
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
HalfSweet
force-pushed
the
jtag-single-bit
branch
from
e5ac440 to
06cf0a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
projects/HSLink-Pro/src/JTAG_DP/JTAG_DP_SPI.c (5)
65-66: Fix the typo in the function name.
The function namejtag_spi_sequeceis misspelled. Consider renaming it tojtag_spi_sequencefor clarity and consistency.-static void jtag_spi_sequece(uint32_t info, const uint8_t *tdi, uint8_t *tdo); +static void jtag_spi_sequence(uint32_t info, const uint8_t *tdi, uint8_t *tdo);
96-98: Validate pull configuration on TDI pin.
Pull enable (PE_SET(1)) combined with pull select set to 0 might add an internal pull-down on the TDI line. If TDI is driven externally, ensure this internal pull is desired.
100-105: Consider refactoring repeated PAD_CTL assignments.
Both SRST and TRST have very similar PAD_CTL configurations. Extracting them into a helper or struct could improve maintainability.
123-125: Minor localized comment style note.
The inline comment is in Chinese. If code comments are typically English in this project, consider adding or switching to an English equivalent for consistency.
410-414: Eliminate unnecessary casts for clarity.
Iftdiandtdoare alreadyuint8_t*, casting to(uint8_t *)is redundant.- JTAG_SPI_BASE->DATA = *(uint8_t *)(tdi); + JTAG_SPI_BASE->DATA = *tdi; ... - *(uint8_t *)(tdo) = JTAG_SPI_BASE->DATA; + *tdo = JTAG_SPI_BASE->DATA;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
projects/HSLink-Pro/src/HID_COMM/hid_comm.cpp(25 hunks)projects/HSLink-Pro/src/JTAG_DP/JTAG_DP_SPI.c(23 hunks)projects/HSLink-Pro/src/setting.cpp(5 hunks)projects/HSLink-Pro/src/setting.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- projects/HSLink-Pro/src/setting.h
- projects/HSLink-Pro/src/setting.cpp
- projects/HSLink-Pro/src/HID_COMM/hid_comm.cpp
🧰 Additional context used
🧬 Code Definitions (1)
projects/HSLink-Pro/src/JTAG_DP/JTAG_DP_SPI.c (3)
projects/HSLink-Pro/src/DAP_config.h (1)
gpiom_configure_pin_control_setting(368-368)projects/HSLink-Pro/src/JTAG_DP/JTAG_DP.h (5)
SPI_JTAG_Sequence(17-17)SPI_JTAG_ReadIDCode(18-18)SPI_JTAG_WriteAbort(19-19)SPI_JTAG_IR(20-20)SPI_JTAG_Transfer(21-21)projects/HSLink-Pro/common/hslinkpro/board.h (1)
board_write_spi_cs(335-335)
🔇 Additional comments (13)
projects/HSLink-Pro/src/JTAG_DP/JTAG_DP_SPI.c (13)
33-41: No issues with the added include statements and spacing.
They appear necessary for clock configuration and do not introduce any apparent conflicts.
69-70: Formatting change looks consistent.
Splitting the return type onto a separate line is stylistically acceptable, and there are no functional issues.
86-87: Confirm the need for loopback mode.
IOC_PAD_FUNC_CTL_LOOP_BACK_SET(1)is enabled here. Verify this is intended for diagnostic or functional purposes, as loopback can introduce unexpected behavior if not strictly required.
120-122: No issues found.
Switching the TMS pin to output here is consistent with JTAG usage.
126-135: Single-bit mode GPIO setup looks logically sound.
TCK and TDI are configured as outputs, and TDO is set as input. Confirm that the rest of the flow properly reverts pins if single-bit mode is later disabled or changed.
144-145: Reformatted function signature is acceptable.
No functionality changes observed.
180-181: Function declaration reformat is benign.
No functional changes introduced.
190-191: No concerns with function declaration rewrite.
Everything else appears consistent.
202-208: Conditional call to fast vs. slow IR update is correct.
Implementation aligns well with the existingJTAG_IR_Fast/Slowpattern.
216-222: Conditional call to fast vs. slow transfer is correct.
Matches the approach used for IR handling.
481-482: Signature reformat is fine.
No functional changes noted.
489-490: GPIO write additions look consistent.
Pin transitions in error-handling and setup blocks appear logically correct.Also applies to: 492-492, 494-494, 519-520, 521-521
576-576: No issues with updated inline comment.
It clarifies the code’s intention during the write transfer block.
Summary by CodeRabbit
This update introduces a new option in device settings that allows you to enable a dedicated JTAG Single-Bit Mode for enhanced hardware interface control and flexibility.
jtag_single_bit_mode.