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.

Sign up for GitHub

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

Jump to bottom

fpga: Add reset controller for FPGA peripherals #371

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fpga: Add reset controller for FPGA peripherals #371

wants to merge 5 commits into from
+449 −25

Conversation

pussuw
Copy link

@pussuw pussuw commented Mar 17, 2025

Needs FPGA + bootloader update

@jlaitine
Copy link

Code looks good in general, I just started wondering about one thing

  • We'd most likely like to up-stream the driver parts, since at least corespi, corei2c and corepwm are publicly available Microchip IPs, and the existing code supports them
  • But the new sysreg / reset register IP is our own, so not relevant for upstreaming
    -> this hard-wires together drivers for publicly available IP blocks and our custom reset logic.

So, IMHO the drivers require to use some generic api & stubs to issue block reset, which can be implemented by whomever designs the FPGA, if reset functionality is required... Or have some CONFIG flag that can be used to leave out the reset functionality.

In other words, as the state of this PR is now, it is not possible to instantiate only publicly available IP blocks on FPGA and take the corresponding CoreXXX drivers into use with those, since the reset interfaces wouldn't work.

@pussuw
Copy link
Author

pussuw commented Mar 18, 2025

Code looks good in general, I just started wondering about one thing

  • We'd most likely like to up-stream the driver parts, since at least corespi, corei2c and corepwm are publicly available Microchip IPs, and the existing code supports them
  • But the new sysreg / reset register IP is our own, so not relevant for upstreaming
    -> this hard-wires together drivers for publicly available IP blocks and our custom reset logic.

So, IMHO the drivers require to use some generic api & stubs to issue block reset, which can be implemented by whomever designs the FPGA, if reset functionality is required... Or have some CONFIG flag that can be used to leave out the reset functionality.

In other words, as the state of this PR is now, it is not possible to instantiate only publicly available IP blocks on FPGA and take the corresponding CoreXXX drivers into use with those, since the reset interfaces wouldn't work.

Yes this code is not upstreamable. It is very specifically stated here as well. If you want a, generic reset and clock control driver is of course an option. That driver should precede this content.

@pussuw pussuw force-pushed the fpga_reset_controller branch from 9450949 to ffd3653 Compare March 19, 2025 09:52
@pussuw
Copy link
Author

pussuw commented Mar 19, 2025

The stub driver is the first patch, that is going to upstream. The rest will stay with us.

pussuw added 2 commits March 19, 2025 11:56
@pussuw
mpfs/mpfs_rcc: Add stub driver for FPGA clock and reset controller
0ff03ad 
This patch adds a driver framework to control individual FPGA reset and
clock gates.

For FPGA devices, only the fabric (FIC0/FIC1/FIC3) clock and reset can be
controlled, which affects the whole fabric / domain. This is problematic
for obvious reasons.

For MSS peripherals, clocks and reset are controlled individually for each
peripheral via MSS memory mapped registers (MPFS_SYSREG). To get the same
capability for FPGA peripherals, the same controller needs to be
fabricated on the FPGA.

However, the FPGA clock/reset controller is entirely user dependent, so a
generic implementation is not possible. However, a generic driver is
needed in order to build the current FPGA peripheral drivers. A stub
implementation of the driver is provided in order to achieve this.

# Conflicts:
#	arch/risc-v/src/mpfs/Make.defs
#	arch/risc-v/src/mpfs/mpfs_corepwm.c
#	arch/risc-v/src/mpfs/mpfs_i2c.c
@pussuw
mpfs/fgpa: Add mpfs_fpga_sysreg which describes the FPGA reset contro…
4ceb11b 
…ller

This adds peripheral reset support for the FPGA peripherals we have.

Note: This is NOT for upstream
@pussuw pussuw force-pushed the fpga_reset_controller branch 2 times, most recently from 2d2b224 to 3a70748 Compare March 19, 2025 10:05
pussuw added 3 commits March 19, 2025 12:16
@pussuw
mpfs_rcc.c: Add implementation for FPGA RCC
0481804 
This driver can control resets for:
- CAN
- UART
- SPI
- I2C
- PWM

Others can be added as needed.
@pussuw
mpfs/fgpa: Release CAN from reset
540cdf8
@pussuw
mpfs/fpga: Release CoreUART from reset
4dd5cb2
@pussuw pussuw force-pushed the fpga_reset_controller branch from 3a70748 to 4dd5cb2 Compare March 19, 2025 10:16
jlaitine
jlaitine approved these changes Mar 19, 2025
View reviewed changes
Copy link

@jlaitine jlaitine left a comment

Choose a reason for hiding this comment

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

LGTM!

@pussuw
Copy link
Author

pussuw commented Mar 19, 2025

I'll do the upstream part first.

jpaali
jpaali approved these changes Mar 21, 2025
View reviewed changes
Copy link

@jpaali jpaali left a comment

Choose a reason for hiding this comment

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

Looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaitine jlaitine jlaitine approved these changes

@jpaali jpaali jpaali approved these changes

@haitomatic haitomatic Awaiting requested review from haitomatic

Assignees
No one assigned
Labels
Arch: risc-v Size: M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pussuw @jlaitine @jpaali