Skip to content

Conversation

@atlomak
Copy link
Contributor

@atlomak atlomak commented Nov 17, 2025

Add dynamic reconfiguration of per port run state frequency divider.

Description

Added new message types in grspwrtr driver, which allows to set and get current value of rtr.idiv register.

Motivation and Context

JIRA: CSAT-225

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

  • This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  • I will merge this PR by myself when appropriate.

Add `MAP_CONTIGUOUS` flags to `mmap` to prevent memory disruption introduced by DMA.

JIRA: CSAT-226
@gemini-code-assist
Copy link

Summary of Changes

Hello @atlomak, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on enhancing the SpaceWire router (grspwrtr) by adding dynamic clock divider configuration per port, which allows for more granular control over SpaceWire link speeds. Concurrently, the SpaceWire library (libgrspw) undergoes significant refactoring to streamline packet transmission logic, introduce timeout capabilities for receive operations, and improve memory allocation for DMA buffers, ultimately leading to a more robust and flexible SpaceWire communication stack.

Highlights

  • Dynamic Clock Divider Configuration: Introduced the ability to dynamically reconfigure the run state frequency divider for SpaceWire ports, allowing for flexible clock speed adjustments per port.
  • SpaceWire Port Validation Enhancements: Improved port validation logic in the SpaceWire router (grspwrtr) by introducing new constants for maximum physical ports and total port count, making port range checks more robust.
  • Unified Transmit Function in libgrspw: Refactored the SpaceWire library (libgrspw) to consolidate spw_transmitWait and spw_transmitAsync into a single spw_transmit function, simplifying the API and internal logic for packet transmission.
  • Enhanced Receive Operation with Timeout: The spw_rxRead function in libgrspw now supports a timeout mechanism, allowing for non-blocking receive operations and proper handling of unacknowledged descriptors upon timeout.
  • Memory Allocation Improvements: Modified buffer allocation for SpaceWire transmit and receive buffers to use MAP_CONTIGUOUS, ensuring physically contiguous memory for DMA operations, and made MAX_PACKET_LEN configurable.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces dynamic reconfiguration for the SpaceWire router's clock divider and refactors the libgrspw library to merge synchronous/asynchronous transmit functions and add receive timeouts. The changes are generally good, but I've identified a few issues. There's a significant logic flaw in port validation within grspwrtr.c that could lead to incorrect behavior. Additionally, there's a minor style inconsistency in a header file and a potential concurrency issue in libgrspw.c related to condition variable signaling. I've provided detailed comments and suggestions to address these points.

@github-actions
Copy link

github-actions bot commented Nov 17, 2025

Unit Test Results

9 457 tests   - 5   8 867 ✅  - 6   51m 32s ⏱️ +38s
  561 suites ±0     589 💤 ±0 
    1 files   ±0       1 ❌ +1 

For more details on these failures, see this check.

Results for commit 3e0cf14. ± Comparison against base commit edea57b.

This pull request removes 6 and adds 1 tests. Note that renamed tests count towards both.
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket.test_unix_socket.accept_connect_liveness
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket.test_unix_socket.flags
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket.test_unix_socket.send_clear_peer_closed
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket.test_unix_socket.wrong_family
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket.test_unix_socket.wrong_port
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket.test_unix_socket.wrong_type
phoenix-rtos-tests/libc/unix-socket ‑ aarch64a53-zynqmp-qemu:phoenix-rtos-tests/libc/unix-socket

♻️ This comment has been updated with latest results.

@atlomak atlomak force-pushed the atlomak/spwrtr-clkdiv branch from c5ee928 to c093972 Compare November 17, 2025 13:21
@atlomak atlomak changed the title Add spwrtr clk div devices/grspwrtr: add clkdiv Nov 18, 2025
@atlomak atlomak force-pushed the atlomak/spwrtr-clkdiv branch from c093972 to 3e0cf14 Compare December 2, 2025 12:27
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