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

risc-v: add Microchip's PolarFire SoC FPGA manager interface #67802

Merged

Conversation

con-pax
Copy link
Contributor

@con-pax con-pax commented Jan 18, 2024

This patch set adds support for the "Auto Update" feature on PolarFire SoC that allows for writing an FPGA bitstream to the SPI flash connected to the system controller.

On power cycle, "Auto Update" will take place, and program the FPGA with the contents of the SPI flash - provided that that image is valid and an actual upgrade from that already programmed.

An important note: this service is not possible on PolarFire SoC engineering sample's, due to a bug on the QSPI controller connected to the system controller.

@zephyrbot zephyrbot added area: RISCV RISCV Architecture (32-bit & 64-bit) area: FPGA Field-Programmable Gate Array (FPGA) platform: Microchip MEC Microchip MEC Platform area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jan 18, 2024
cfriedt
cfriedt previously approved these changes Jan 18, 2024
Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

Love this! When does Zephyr go to space again??

Copy link
Member

@fkokosinski fkokosinski left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your PR! I've left some comments

Comment on lines 89 to 90
compatible = "microchip,mpfs-fpga";
status = "disabled";
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: this indentation looks wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkokosinski a fix has been implemented

Comment on lines 25 to 27
#define MSS_SCBMAILBOX ((volatile uint32_t *)(0x37020800UL))
#define MSS_SCB_SERVICES_CR (*(volatile uint32_t *)(0x37020050UL))
#define MSS_SCB_SERVICES_SR (*(volatile uint32_t *)(0x37020054UL))
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should be reading these addresses from the device tree. Maybe a syscon node/driver would be a good fit here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. We will look at this, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carlocaione @fkokosinski
Thanks for this one guys, we have went with a different approach

#define MSS_SCB_SERVICES_SR (*(volatile uint32_t *)(0x37020054UL))

#define SCBCTRL_SERVICESCR_REQ (0u)
#define SCBCTRL_SERVICESCR_REQ_MASK (1u << SCBCTRL_SERVICESCR_REQ)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define SCBCTRL_SERVICESCR_REQ_MASK (1u << SCBCTRL_SERVICESCR_REQ)
#define SCBCTRL_SERVICESCR_REQ_MASK BIT(SCBCTRL_SERVICESCR_REQ)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkokosinski thanks! we will fix this

#define SCBCTRL_SERVICESCR_REQ_MASK (1u << SCBCTRL_SERVICESCR_REQ)

#define SCBCTRL_SERVICESSR_BUSY (1u)
#define SCBCTRL_SERVICESSR_BUSY_MASK (1u << SCBCTRL_SERVICESSR_BUSY)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define SCBCTRL_SERVICESSR_BUSY_MASK (1u << SCBCTRL_SERVICESSR_BUSY)
#define SCBCTRL_SERVICESSR_BUSY_MASK BIT(SCBCTRL_SERVICESSR_BUSY)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkokosinski thanks! we will fix this one too

#define SCBCTRL_SERVICESSR_BUSY_MASK (1u << SCBCTRL_SERVICESSR_BUSY)

#define SCBCTRL_SERVICESSR_STATUS (16u)
#define SCBCTRL_SERVICESSR_STATUS_MASK (0xFFFFu << SCBCTRL_SERVICESSR_STATUS)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define SCBCTRL_SERVICESSR_STATUS_MASK (0xFFFFu << SCBCTRL_SERVICESSR_STATUS)
#define SCBCTRL_SERVICESSR_STATUS_MASK GENMASK(SCBCTRL_SERVICESSR_STATUS + 3, SCBCTRL_SERVICESSR_STATUS)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkokosinski, excellent, thanks for the pointing out that macro

fkokosinski
fkokosinski previously approved these changes Jan 25, 2024
@con-pax
Copy link
Contributor Author

con-pax commented Jan 31, 2024

Love this! When does Zephyr go to space again??

Hey @cfriedt,
could I pester you once more for re-approval? would love to get this in for v3.6 and I believe there is a feature freeze on the horizon (2nd Feb)
Thanks a million!

cfriedt
cfriedt previously approved these changes Jan 31, 2024
@con-pax
Copy link
Contributor Author

con-pax commented Jan 31, 2024

I will go fix the conflicts on this now 😃

Add support for Microchip's PolarFire SoC system controller QSPI
interface.

Signed-off-by: Harshit Agarwal <harshit.agarwal@microchip.com>
Add Microchip's PolarFire SoC mailbox node.

Signed-off-by: Harshit Agarwal <harshit.agarwal@microchip.com>
Add FPGA driver support for Microchip PolarFire SoC.

Signed-off-by: Harshit Agarwal <harshit.agarwal@microchip.com>
Microchip's PolarFire SoC interface with on-board spi
nor flash via system controller. This on-board spi nor flash can be
used to store FPGA design bitstream's.

Signed-off-by: Harshit Agarwal <harshit.agarwal@microchip.com>
@cfriedt
Copy link
Member

cfriedt commented Feb 1, 2024

@fkokosinski, @kgugala - can either of you add a +1 after this was rebased to fix merge conflicts?

@fkokosinski
Copy link
Member

@cfriedt done :)

@cfriedt cfriedt merged commit 15b4975 into zephyrproject-rtos:main Feb 1, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: FPGA Field-Programmable Gate Array (FPGA) area: mbox area: RISCV RISCV Architecture (32-bit & 64-bit) platform: Microchip MEC Microchip MEC Platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants