Skip to content

hal/armv7a/imx6ull: add pctl for RTC config#751

Draft
rmikielis wants to merge 1 commit intomasterfrom
rmikielis/imx6ull-rtc_pctl
Draft

hal/armv7a/imx6ull: add pctl for RTC config#751
rmikielis wants to merge 1 commit intomasterfrom
rmikielis/imx6ull-rtc_pctl

Conversation

@rmikielis
Copy link
Copy Markdown
Contributor

SEN-92

Description

Since SNVS is in secure domain, only privileged software can change its state. Adding basic pctl
for secure RTC enabling/disabling was necessary

Tested and working, but please review if adding snvs registers to kernel page tables is done properly.

Motivation and Context

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?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (sensorbox).

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.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, 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 introduces the necessary infrastructure to manage the Secure Non-Volatile Storage (SNVS) Real-Time Clock (RTC) on the i.MX6ULL platform. It enables privileged software to programmatically control the RTC's state, addressing the security requirement that SNVS components reside in a secure domain. The changes involve memory mapping, register definitions, and a new platform control interface for RTC operations.

Highlights

  • SNVS Register Mapping: The _init.S file was updated to map the Secure Non-Volatile Storage (SNVS) registers into the kernel page tables, making them accessible for privileged operations.
  • SNVS Register Definitions: New enum definitions for SNVS registers were added in imx6ull.c to provide symbolic names for register offsets, improving code readability and maintainability.
  • RTC Control Function: A new static function, _imx6ull_setRTC, was implemented to enable or disable the Real-Time Clock (RTC) within the SNVS module, including a timeout mechanism for state changes.
  • Platform Control Integration: The hal_platformctl function was extended to include a new pctl_snvs type, allowing privileged software to control the SNVS RTC state through the platform control interface.
  • Platform Control Structure Update: The platformctl_t structure in imx6ull.h was modified to include the pctl_snvs type and a corresponding union member for passing RTC state values, enabling the new control functionality.
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.

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.

@rmikielis rmikielis changed the title hal/armv7a/imx6ull: add pctl for RCT config hal/armv7a/imx6ull: add pctl for RTC config Mar 19, 2026
Copy link
Copy Markdown

@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

The pull request introduces support for the i.MX6ULL's Secure Non-Volatile Storage (SNVS) and its Real-Time Clock (RTC). This involves mapping SNVS registers during system initialization, defining SNVS register offsets, adding SNVS register access to the platform's common structure, and implementing a function to control the RTC's enable/disable state. This RTC control is exposed through the hal_platformctl interface. A review comment notes that the newly introduced pctl_rtc_set_state enum in imx6ull.h is currently unused and should either be removed or refactored for better clarity.

Comment on lines +207 to +210
/* SNVS domain */
enum {
pctl_rtc_set_state
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The new enum pctl_rtc_set_state is defined but not used anywhere in the codebase. It seems to add confusion without providing functionality. If the intention is to define constants for enabling/disabling the RTC, it would be better to define them explicitly (e.g., RTC_STATE_DISABLE = 0, RTC_STATE_ENABLE = 1). As it is, this enum should probably be removed to avoid confusion.

@github-actions
Copy link
Copy Markdown

Unit Test Results

9 553 tests  ±0   8 961 ✅ ±0   53m 14s ⏱️ +30s
  591 suites ±0     592 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 0ad6410. ± Comparison against base commit cbfa5d0.

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.

1 participant