lkml.org 
[lkml]   [2017]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 2/4] fpga pr ip: Core driver support for Altera Partial Reconfiguration IP.


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.gerlach@linux.intel.com matthew.gerlach@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
>

\
 
 \ /
  Last update: 2017-03-21 00:53    [W:0.366 / U:0.100 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site