Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 8 Jun 2017 02:38:55 +0300 | Subject | Re: [PATCH v5] fpga manager: Add Altera CvP driver |
| |
On Thu, Jun 8, 2017 at 2:09 AM, Anatolij Gustschin <agust@denx.de> wrote: > On Fri, 2 Jun 2017 20:43:21 +0300 > Andy Shevchenko andy.shevchenko@gmail.com wrote:
Besides below comments, please, do
s/VSEC_/VSE_/g
for entire file.
We are following PCI and Thunderbolt pattern for use of Vendor Specific Extended Capability.
>>> + if (!timeout_us) >>> + return -ETIMEDOUT; >> >>Hmm... >>What as a user I would expect here is at least one attempt (0 -- no >>timeout, but try once). > > yes, this first attempt is above, please see original patch for full > context.
Ah, it means you don't correctly use do {} while approach.
Remove everything above do { and move usleep after check for the status inside the loop.
>>> + /* clock to data ratio for uncompressed and unencrypted images */ >>> + conf->numclks = 1; >> >>To else branch of below? >> >>> + if (info) { >> >>> + } > > then, the default numclks value would have to be set in the if branch > as well. It is easier to set it before.
See the magic below:
u32 iflags = info ? info->flags : 0; ... if (iflags & FPGA_MGR_PARTIAL_RECONFIG) { dev_err(&mgr->dev, "Partial reconfiguration not supported.\n"); return -EINVAL; }
if (iflags & FPGA_MGR_COMPRESSED_BITSTREAM) conf->numclks = 8; /* ratio for all compressed images */ else if (iflags & FPGA_MGR_ENCRYPTED_BITSTREAM) conf->numclks = 4; /* ratio for encrypted images */ else conf->numclks = 1; /* clock to data ratio for uncompressed and unencrypted images */
I would really recommend to double check the code every time you are about to send. A little thought can make a beauteful result!
>>> +static struct pci_device_id altera_cvp_id_tbl[] = { >>> + { PCI_VDEVICE(ALTERA, PCI_ANY_ID) }, >> >>Does it have dedicated PCI class? >> >>PCI_ANY_ID usually is too broad. > > no, it doesn't have dedicated class.
Hmm... It means any device of this vendor will jump into this driver... Not good.
-- With Best Regards, Andy Shevchenko
| |