Re: [PATCH] fpga: altera-cvp: Add Agilex5 CVP credit register support
From: Xu Yilun
Date: Fri Jun 26 2026 - 12:43:32 EST
On Thu, Jun 18, 2026 at 08:43:27PM -0700, muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
>
> Extend the CvP driver to support Agilex5, which uses a different
> register layout than previous SoC FPGA variants for CVP bitstream
> transfer.
>
> Agilex5 uses a different VSEC layout (VSEC length 0x60) and a
> dedicated CVP credit register at offset 0x5C with 12-bit width,
> compared to offset 0x49 with 8-bit width on other platforms.
>
> Detect the device family at probe time by reading the VSEC specific
> header and checking the VSEC length field. Introduce
> device_family_type to distinguish V1, V2, and V2 Agilex5 variants.
> Use altera_read_config_dword() with a 0xFFF mask for Agilex5 credits
> and altera_read_config_byte() for all other devices.
> Fix the credit
> comparison by removing the incorrect u8 cast on sent_packets.
Is it a bug fix for existing cases? If yes, split it out to a separate
patch. Otherwise, don't say 'fix'.
>
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
> ---
> drivers/fpga/altera-cvp.c | 48 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> index 44badfd11e1b..58128de2a8c5 100644
> --- a/drivers/fpga/altera-cvp.c
> +++ b/drivers/fpga/altera-cvp.c
> @@ -10,6 +10,7 @@
> * Firmware must be in binary "rbf" format.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/fpga/fpga-mgr.h>
> @@ -22,6 +23,9 @@
> #define TIMEOUT_US 2000 /* CVP STATUS timeout for USERMODE polling */
>
> /* Vendor Specific Extended Capability Registers */
> +#define VSE_PCIE_SPECIFIC_HEADER 0x4 /* VSEC ID, Revision, length */
> +#define VSEC_LENGTH GENMASK(31, 20)
Are they PCIe standard registers?
> +#define AGILEX5_VSEC_LENGTH 0x60 /* Agilex5 only */
> #define VSE_CVP_STATUS 0x1c /* 32bit */
> #define VSE_CVP_STATUS_CFG_RDY BIT(18) /* CVP_CONFIG_READY */
> #define VSE_CVP_STATUS_CFG_ERR BIT(19) /* CVP_CONFIG_ERROR */
> @@ -48,6 +52,7 @@
> #define V1_VSEC_OFFSET 0x200 /* Vendor Specific Offset V1 */
> /* V2 Defines */
> #define VSE_CVP_TX_CREDITS 0x49 /* 8bit */
> +#define VSE_CVP_AG5_TX_CREDITS 0x5C /* 12bit credits for Agilex5 */
>
> #define V2_CREDIT_TIMEOUT_US 40000
> #define V2_CHECK_CREDIT_US 10
> @@ -58,6 +63,9 @@
>
> #define DRV_NAME "altera-cvp"
> #define ALTERA_CVP_MGR_NAME "Altera CvP FPGA Manager"
> +#define SOCFPGA_CVP_V1_OTHERS 0x1
> +#define SOCFPGA_CVP_V2_OTHERS 0x2
I didn't see they are used anywhere.
> +#define SOCFPGA_CVP_V2_AGILEX5 0x3
>
> /* Write block sizes */
> #define ALTERA_CVP_V1_SIZE 4
> @@ -78,6 +86,7 @@ struct altera_cvp_conf {
> u32 sent_packets;
> u32 vsec_offset;
> const struct cvp_priv *priv;
> + u32 device_family_type;
> };
>
> struct cvp_priv {
> @@ -228,18 +237,32 @@ static int altera_cvp_v2_wait_for_credit(struct fpga_manager *mgr,
> u32 timeout = V2_CREDIT_TIMEOUT_US / V2_CHECK_CREDIT_US;
> struct altera_cvp_conf *conf = mgr->priv;
> int ret;
> - u8 val;
> + u32 val;
> + u32 credit_mask = GENMASK(7, 0);
> + u32 vse_cvp_tx_credits_offset = VSE_CVP_TX_CREDITS;
I'm not strict on reverse xmas tree, but don't make a complete mess.
> +
> + if (conf->device_family_type == SOCFPGA_CVP_V2_AGILEX5) {
> + vse_cvp_tx_credits_offset = VSE_CVP_AG5_TX_CREDITS;
> + credit_mask = GENMASK(11, 0);
> + }
>
> do {
> - ret = altera_read_config_byte(conf, VSE_CVP_TX_CREDITS, &val);
> + /* READ DWORD is required for Agilex5 but READ BYTE is required for non-Agilex5 */
> + if (conf->device_family_type == SOCFPGA_CVP_V2_AGILEX5)
> + ret = altera_read_config_dword(conf, vse_cvp_tx_credits_offset, &val);
> + else
> + ret = altera_read_config_byte(conf, vse_cvp_tx_credits_offset, (u8 *)&val);
> +
> if (ret) {
> dev_err(&conf->pci_dev->dev,
> "Error reading CVP Credit Register\n");
> return ret;
> }
>
> + val = val & credit_mask;
Don't you have struct cvp_priv to describe HW differences? Why not use
it? Why Bifurcating code like this?
> +
> /* Return if there is space in FIFO */
> - if (val - (u8)conf->sent_packets)
> + if (val - conf->sent_packets)
> return 0;
>
> ret = altera_cvp_chk_error(mgr, blocks * ALTERA_CVP_V2_SIZE);
> @@ -507,6 +530,8 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
> conf->priv->user_time_us);
> if (ret)
> dev_err(&mgr->dev, "PLD_CLK_IN_USE|USERMODE timeout\n");
> + else
> + dev_notice(&mgr->dev, "CVP write completed successfully.\n");
Why adding this print, anything to do with "support Agilex5"?
>
> return ret;
> }
> @@ -627,10 +652,23 @@ static int altera_cvp_probe(struct pci_dev *pdev,
> conf->pci_dev = pdev;
> conf->write_data = altera_cvp_write_data_iomem;
>
> - if (conf->vsec_offset == V1_VSEC_OFFSET)
> + /* To differentiate the target SOCFPGA */
> + if (conf->vsec_offset == V1_VSEC_OFFSET) {
> conf->priv = &cvp_priv_v1;
> - else
> + conf->device_family_type = SOCFPGA_CVP_V1_OTHERS;
Why add another board type indicator, conf->priv won't work?
> + dev_notice(&pdev->dev, "V1 target SOCFPGA detected.\n");
No news is good news, a good driver keeps quiet.
> + } else {
> + /* Agilex7, Stratix10, Agilex5*/
> conf->priv = &cvp_priv_v2;
> + pci_read_config_dword(pdev, offset + VSE_PCIE_SPECIFIC_HEADER, ®val);
> + if (FIELD_GET(VSEC_LENGTH, regval) == AGILEX5_VSEC_LENGTH) {
> + conf->device_family_type = SOCFPGA_CVP_V2_AGILEX5;
> + dev_notice(&pdev->dev, "V2 target SOCFPGA Agilex5 detected.\n");
> + } else {
> + conf->device_family_type = SOCFPGA_CVP_V2_OTHERS;
> + dev_notice(&pdev->dev, "V2 target SOCFPGA detected.\n");
> + }
> + }
BTW: I really suggest you don't send multiple threads at a time. Getting the
first patch right, then I'll have confidence to move to the next.
>
> conf->map = pci_iomap(pdev, CVP_BAR, 0);
> if (!conf->map) {
> --
> 2.43.7
>
>