[PATCH 1/1] drivers: fpga: Add partial reconfiguration console commands

[Michal Simek]

On 3/14/25 12:47, Naresh Kumar Ravulapalli wrote:
> Partial Reconfiguration (pr) command is added to U-Boot console. The
> pr command will use the Freeze Controller which can freeze and
> unfreeze the specified partial reconfiguration region. The pr command
> supports multiple regions for partial reconfiguration by specifying
> the region ID.
> 
> Signed-off-by: Naresh Kumar Ravulapalli <nareshkumar.ravulapalli at altera.com>
> ---
>   drivers/fpga/Kconfig    |   9 ++
>   drivers/fpga/Makefile   |   1 +
>   drivers/fpga/intel_pr.c | 191 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 201 insertions(+)
>   create mode 100644 drivers/fpga/intel_pr.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 62cb77b098..bf34d11a9d 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -66,6 +66,15 @@ config FPGA_LATTICE
>   	  This is used for the lattice FPGAs. Please check the source code as
>   	  there is no documentation for this at present.
>   
> +config FPGA_INTEL_PR
> +	bool "Enable Intel FPGA Partial Reconfiguration driver"
> +	depends on FPGA_ALTERA
> +	help
> +	  Say Y here to enable the Intel FPGA Partial Reconfiguration driver
> +
> +	  This provides basic functionality for partial reconfiguration which
> +	  include the freeze controller support.
> +
>   config FPGA_XILINX
>   	bool "Enable Xilinx FPGA drivers"
>   	select FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 610c168fc3..70a8f88836 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -25,4 +25,5 @@ obj-$(CONFIG_FPGA_STRATIX_V) += stratixv.o
>   obj-$(CONFIG_FPGA_SOCFPGA) += socfpga.o
>   obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += socfpga_gen5.o
>   obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += socfpga_arria10.o
> +obj-$(CONFIG_FPGA_INTEL_PR) += intel_pr.o
>   endif
> diff --git a/drivers/fpga/intel_pr.c b/drivers/fpga/intel_pr.c
> new file mode 100644
> index 0000000000..ecca2c9eeb
> --- /dev/null
> +++ b/drivers/fpga/intel_pr.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Intel Corporation

2018? That's quite old file.

> + */
> +
> +#include <command.h>
> +#include <errno.h>
> +#include <wait_bit.h>
> +#include <asm/global_data.h>
> +#include <asm/io.h>
> +#include <linux/bitops.h>
> +#include <linux/libfdt.h>
> +#include <fdtdec.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define FREEZE_CSR_STATUS_OFFSET		0
> +#define FREEZE_CSR_CTRL_OFFSET			4
> +#define FREEZE_CSR_ILLEGAL_REQ_OFFSET		8
> +#define FREEZE_CSR_REG_VERSION			12
> +
> +#define FREEZE_TIMEOUT				20000
> +
> +#define FREEZE_CSR_STATUS_FREEZE_REQ_DONE	BIT(0)
> +#define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE	BIT(1)
> +
> +#define FREEZE_CSR_CTRL_FREEZE_REQ		BIT(0)
> +#define FREEZE_CSR_CTRL_RESET_REQ		BIT(1)
> +#define FREEZE_CSR_CTRL_UNFREEZE_REQ		BIT(2)
> +
> +static int intel_get_freeze_br_addr(fdt_addr_t *addr, unsigned int region)
> +{
> +	int offset;
> +	char freeze_br[12];
> +	struct fdt_resource r;
> +	int ret;
> +
> +	snprintf(freeze_br, sizeof(freeze_br), "freeze_br%d", region);

_ is not allowed char for alias.

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/aliases.yaml#L30

> +
> +	const char *alias = fdt_get_alias(gd->fdt_blob, freeze_br);
> +
> +	if (!alias)	{
> +		printf("alias %s not found in dts\n", freeze_br);
> +		return -ENODEV;
> +	}
> +
> +	offset = fdt_path_offset(gd->fdt_blob, alias);
> +	if (offset < 0) {
> +		printf("%s not found in dts\n", alias);
> +		return -ENODEV;
> +	}
> +
> +	ret = fdt_get_resource(gd->fdt_blob, offset, "reg", 0, &r);
> +	if (ret) {
> +		printf("%s has no 'reg' property!\n", freeze_br);
> +		return ret;
> +	}
> +
> +	*addr = r.start;
> +
> +	return ret;
> +}
> +
> +static int intel_freeze_br_req_ack(fdt_addr_t addr, u32 req_ack)
> +{
> +	u32 status, illegal, ctrl;
> +	int ret = -ETIMEDOUT;
> +	unsigned long start = get_timer(0);
> +
> +	while (1) {
> +		illegal = readl(addr + FREEZE_CSR_ILLEGAL_REQ_OFFSET);
> +		if (illegal) {
> +			printf("illegal request 0x%08x detected in freeze bridge\n", illegal);
> +
> +			writel(illegal, addr + FREEZE_CSR_ILLEGAL_REQ_OFFSET);
> +
> +			illegal = readl(addr + FREEZE_CSR_ILLEGAL_REQ_OFFSET);
> +			if (illegal)
> +				printf("illegal request 0x%08x detected in freeze bridge are not cleared\n",
> +				       illegal);
> +
> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		status = readl(addr + FREEZE_CSR_STATUS_OFFSET);
> +		status &= req_ack;
> +		if (status) {
> +			ctrl = readl(addr + FREEZE_CSR_CTRL_OFFSET);
> +			printf("%s request %x acknowledged %x %x\n",
> +			       __func__, req_ack, status, ctrl);
> +
> +			ret = 0;
> +			break;
> +		}
> +
> +		if (get_timer(start) > FREEZE_TIMEOUT)
> +			break;
> +
> +		udelay(1);
> +		schedule();
> +	}
> +
> +	return ret;
> +}
> +
> +static int intel_freeze_br_do_freeze(unsigned int region)
> +{
> +	u32 status;
> +	int ret;
> +	fdt_addr_t addr;
> +
> +	ret = intel_get_freeze_br_addr(&addr, region);
> +	if (ret)
> +		return ret;
> +
> +	status = readl(addr + FREEZE_CSR_STATUS_OFFSET);
> +
> +	if (status & FREEZE_CSR_STATUS_FREEZE_REQ_DONE)
> +		return 0;

newline

> +	else if (!(status & FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE))

no reason for this else.

> +		return -EINVAL;
> +
> +	writel(FREEZE_CSR_CTRL_FREEZE_REQ, addr + FREEZE_CSR_CTRL_OFFSET);
> +
> +	ret = intel_freeze_br_req_ack(addr, FREEZE_CSR_STATUS_FREEZE_REQ_DONE);
> +	if (ret)

Don't you want to show the reason for this failure at least in debug?

> +		writel(0, addr + FREEZE_CSR_CTRL_OFFSET);
> +	else
> +		writel(FREEZE_CSR_CTRL_RESET_REQ, addr + FREEZE_CSR_CTRL_OFFSET);
> +
> +	return ret;
> +}
> +
> +static int intel_freeze_br_do_unfreeze(unsigned int region)
> +{
> +	u32 status;
> +	int ret;
> +	fdt_addr_t addr;
> +
> +	ret = intel_get_freeze_br_addr(&addr, region);
> +	if (ret)
> +		return ret;
> +
> +	writel(0, addr + FREEZE_CSR_CTRL_OFFSET);
> +
> +	status = readl(addr + FREEZE_CSR_STATUS_OFFSET);
> +
> +	if (status & FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE)
> +		return 0;
> +	else if (!(status & FREEZE_CSR_STATUS_FREEZE_REQ_DONE))

again no reason for else here.

> +		return -EINVAL;
> +
> +	writel(FREEZE_CSR_CTRL_UNFREEZE_REQ, addr + FREEZE_CSR_CTRL_OFFSET);
> +
> +	ret = intel_freeze_br_req_ack(addr, FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE);
> +
> +	writel(0, addr + FREEZE_CSR_CTRL_OFFSET);
> +
> +	return ret;
> +}
> +
> +static int do_pr(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	const char *cmd;
> +	char *region;
> +	unsigned int region_num = 0;
> +
> +	if (argc != 2 && argc != 3)
> +		return CMD_RET_USAGE;
> +
> +	cmd = argv[1];
> +	if (argc == 3)
> +		region_num = simple_strtoul(argv[2], &region, 0);
> +
> +	if (strcmp(cmd, "start") == 0)
> +		return intel_freeze_br_do_freeze(region_num);
> +	else if (strcmp(cmd, "end") == 0)
> +		return intel_freeze_br_do_unfreeze(region_num);
> +
> +	return CMD_RET_USAGE;
> +}
> +
> +U_BOOT_CMD(
> +	pr, 3, 1, do_pr,
> +	"SoCFPGA partial reconfiguration (pr) control",
> +	"start [region ID]\n"
> +	"	- start the pr by freezing the region\n"
> +	"end [region ID]\n"
> +	"	- end the pr by unfreezing the PR region\n"
> +);

I don't think this is the reasonable way how to wire it up. We already have fpga 
command and this should be the part of it.

Thanks,
Michal