linux-kernel - Re: [PATCH v5 2/4] fpga pr ip: Core driver support for Altera Partial Reconfiguration IP.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170318195356.19269415@crub>
Date:   Sat, 18 Mar 2017 19:53:56 +0100
From:   Anatolij Gustschin <agust@...x.de>
To:     matthew.gerlach@...ux.intel.com
Cc:     atull@...nel.org, moritz.fischer@...us.com,
        linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, robh+dt@...nel.org,
        mark.rutland@....com
Subject: Re: [PATCH v5 2/4] fpga pr ip: Core driver support for Altera
 Partial Reconfiguration IP.

Hi Matthew,

thanks for the patches. Please see some comments below.

On Fri, 10 Mar 2017 11:40:25 -0800
matthew.gerlach@...ux.intel.com matthew.gerlach@...ux.intel.com wrote:

...
>+	if (!(info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
>+		pr_err("%s Partial Reconfiguration flag not set\n", __func__);

please use dev_err() here.

...
>+	if (val & ALT_PR_CSR_PR_START) {
>+		pr_err("%s Partial Reconfiguration already started\n",

dev_err(), too.

...
>+static int alt_pr_fpga_write_complete(struct fpga_manager *mgr,
>+				      struct fpga_image_info *info)
>+{
>+	u32 i;
>+
>+	for (i = 0; i < info->config_complete_timeout_us; i++) {
>+		switch (alt_pr_fpga_state(mgr)) {
>+		case FPGA_MGR_STATE_WRITE_ERR:
>+			return -EIO;
>+
>+		case FPGA_MGR_STATE_OPERATING:
>+			dev_info(&mgr->dev,
>+				 "successful partial reconfiguration\n");
>+			return 0;
>+
>+		default:
>+			break;
>+		}
>+		udelay(1);
>+	}
>+	dev_err(&mgr->dev, "timed out waiting for write to complete\n");
>+	return -ETIMEDOUT;
>+}

we will always get timed out error if info->config_complete_timeout_us
is zero. Can we change to
	u32 i = 0;
	...
	do {
		...
	} while (info->config_complete_timeout_us > i++);
?

...
>diff --git a/drivers/fpga/altera-pr-ip-core.h b/drivers/fpga/altera-pr-ip-core.h
>new file mode 100644
>index 0000000..3810a90
>--- /dev/null
>+++ b/drivers/fpga/altera-pr-ip-core.h

Should we move this header to include/linux/? We can use register/
unregister functions in other drivers (PCIe) outside drivers/fpga
then.

Thanks,

Anatolij