Skip to content
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

zynq-mpsoc: add support for pll #15701

Merged
merged 1 commit into from
Jan 27, 2025
Merged

zynq-mpsoc: add support for pll #15701

merged 1 commit into from
Jan 27, 2025

Conversation

zouboan
Copy link
Contributor

@zouboan zouboan commented Jan 26, 2025

Summary

The PS clocking system generates clocks for the processors, peripherals, interconnect, and
other system elements. There are five system PLLs to generate high-frequency signals that
are used as clock sources for the several dozen clock generators in the LPD and FPD.
Two system PLL clock units are in the LPD and three are in the FPD power domain. Each PLL
unit has two clock dividers on its output; one in the LPD and one in the FPD. These clock
dividers can provide two different clocking frequencies from one PLL (in the two clock
domains).
Each system PLL unit has a suggested usage, but the individual clock generators can select
from one of the three PLL clocks routed to it as defined by the registers listed in the Clock
Generator Control Registers section.The system PLL units reside in the LPD and FPD power domains:.
Low power domain system PLLs:
°I/O PLL (IOPLL): provides clocks for all low speed peripherals and part of the
interconnect.
°RPU PLL (RPLL): provides clocks for the RPU MPCore and part of the interconnect.
Full-power domain system PLLs:
°APU PLL (APLL): provides clocks for the APU MPCore clock and part of the
interconnect.
°Video PLL (VPLL): provides clocks for the video I/O.
°DDR PLL (DPLL): provides clocks for the DDR controller and part of the interconnect.

Impact

This PR just Impact the UART of ZYNQ MPSOC presently

Testing

tools/configure.sh zcu111:nsh
make

  • Ready to Boot Primary CPU
  • Boot from EL3
  • Boot to C runtime for OS Initialize
    nx_start: Entry
    up_allocate_heap: heap_start=0x0x185000, heap_size=0x7fd7b000
    gic_validate_dist_version: GICv2 detected
    uart_register: Registering /dev/console
    uart_register: Registering /dev/ttyS0
    work_start_highpri: Starting high-priority kernel worker thread(s)
    nxtask_activate: hpwork pid=1,TCB=0x1855d0
    nx_start_application: Starting init thread
    task_spawn: name=nsh_main entry=0x11cde4 file_actions=0 attr=0x183cb0 argv=0x180
    nxtask_activate: nsh_main pid=2,TCB=0x187ad0
    lib_cxx_initialize: _sinit: 0x15f6a8 _einit: 0x15f6a8

NuttShell (NSH) NuttX-10.2.0
nsh> nx_start: CPU0: Beginning Idle Loop
nsh>

@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Board: arm64 Size: L The size of the change in this PR is large labels Jan 26, 2025
@nuttxpr
Copy link

nuttxpr commented Jan 26, 2025

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

This PR description does not fully meet the NuttX requirements. Here's why and how to fix it:

Missing Information in Summary:

  • Why is the change necessary? The description explains the PS clocking system in detail, but it doesn't say why it's being changed. Is there a bug? Is this a performance improvement? Is this adding support for a new clock configuration? This is the most crucial piece of information.
  • What functional part is being changed? While it mentions the Zynq MPSOC UART, it doesn't explain how the changes to the clocking system relate to the UART. Be explicit: "This change modifies the clock source selection for the UART on the Zynq MPSOC..."
  • How does the change exactly work? Instead of describing the clocking system in general, describe the specific code changes. For example, "This change modifies the zynqmp_clockconfig.c file to select the IOPLL as the clock source for the UART by setting the UART_CLK_SRC register to IOPLL_CLK."

Missing Information in Impact:

  • While it mentions impacting the UART, it needs to answer the specific questions with YES/NO and provide details. For instance:
    • Impact on user: YES. Users may need to reconfigure their UART settings if they were relying on a different clock source. Describe what they need to do.
    • Impact on build: Probably NO, but confirm. If YES, explain any changes to Kconfig, Makefiles, etc.
    • Impact on hardware: YES. This impacts the Zynq MPSOC. Be specific: which Zynq MPSOC variants? Does it impact other hardware?
    • Impact on documentation: Probably YES. If the clocking configuration changes, the documentation should be updated. Specify which files.
    • Impact on security: Probably NO, but analyze if there are any potential security implications due to clock changes.
    • Impact on compatibility: Potentially YES. Does this break compatibility with any existing configurations?

Insufficient Testing Information:

  • Build Host(s): Provide details. "Linux Ubuntu 20.04, x86_64, GCC 9.4.0" for example.
  • Target(s): Be specific: "Zynq UltraScale+ MPSoC ZCU111 board".
  • Testing logs before change: The provided logs only show the boot process. Show logs related to the UART functionality before the change. For example, output from a UART loopback test.
  • Testing logs after change: Show the same UART-related tests after the change to demonstrate the effect of the modification.

Example of Improved Summary:

This change fixes a bug where the Zynq MPSOC UART was using an unstable clock source, leading to data corruption at high baudrates. It modifies the clock source selection in zynqmp_clockconfig.c to use the IOPLL instead of the default clock source for the UART peripherals. This is done by setting the UART_CLK_SRC register to IOPLL_CLK during system initialization. This addresses issue #[NuttX issue number].

By providing these missing details, the PR will be much clearer, easier to review, and more likely to be accepted. Remember to be concise and focus on the changes introduced by this PR.

@xiaoxiang781216 xiaoxiang781216 merged commit 1452f32 into apache:master Jan 27, 2025
12 checks passed
@zouboan zouboan deleted the fft branch January 29, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm64 Issues related to ARM64 (64-bit) architecture Board: arm64 Size: L The size of the change in this PR is large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants