On Sat, 18 Mar 2017, Anatolij Gustschin wrote:
Hi Matthew,
Hi Anatolij,
More good feedback. See below.
thanks for the patches. Please see some comments below.
On Fri, 10 Mar 2017 11:40:25 -0800
matthew.gerl...@linux.intel.com matthew.gerl...@linux.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.
Good catch on both of these pr_err(). dev_err() would be better.
...
+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++);
?
This seems more than reasonable to me.
...
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
