From: Andreas Puhm <email@example.com> To: Anatolij Gustschin <firstname.lastname@example.org> Cc: Moritz Fischer <email@example.com>, Alan Tull <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org> Subject: AW: [PATCH] fpga: altera_cvp: restrict registration to CvP enabled devices Date: Tue, 23 Oct 2018 18:46:47 +0000 [thread overview] Message-ID: <3773264cdfcb4c258cc7eebd213302dd@SRV177.busymouse24.de> (raw) In-Reply-To: <20181023182649.047f1f93@crub> Hi Anatolij, > Von: Anatolij Gustschin [mailto:email@example.com] > Gesendet: Dienstag, 23. Oktober 2018 18:27 > An: Andreas Puhm <firstname.lastname@example.org> > Cc: Moritz Fischer <email@example.com>; Alan Tull <firstname.lastname@example.org>; email@example.com; firstname.lastname@example.org > Betreff: Re: [PATCH] fpga: altera_cvp: restrict registration to CvP enabled devices > > Hi Andreas, > > On Mon, 22 Oct 2018 13:15:34 +0000 > Andreas Puhm email@example.com wrote: > ... > >Full description: > >The altera_cvp probe function only checks, > >if the Altera/Intel PCI device configuration space contains a vendor > >specific entry (VSEC Capability Header 0x000b) at offset 0x200. > > But the probe function does not verify, if the PCI device (and further > >the FPGA), for which it has been called, actually supports the Configure- > >via-Protocol feature. > > > >The PCI device (FPGA) can explicitly disable the Configur-via-Protocol > >(CvP) feature by setting the CVP_EN bit, index 20 of CVP_STATUS register, > >to '0'. > >As the altera_cvp probe function does not check this it registers the > >device in any way. > > The CvP docs says that on some FPGAs (e.g. Arria 10) the assertion of CVP > status can take up to 500ms. However it is not clear whether this delay > might be required after peripheral image configuration and after PCIe > link activation. The diagram describing configuration sequence suggests > that CVP_EN should be polled until it is asserted. I can imaging the > situation that this bit is still not asserted when the device is being > probed. Maybe we should better defer device probing if CVP_EN bit is > cleared? When deferred probing fails again and sufficient period for > CVP_EN bit assertion elapsed, then stop deferred probing and return > -ENODEV? > > Thanks, > > Anatolij Anatolij, thank you for your feedback. My rationale behind the patch is as follows: The CVP_EN is part of the Hard PCIe IP core configuration, and therefore, has a defined and static value right from "the start". Remark in [1, fig 12] " For high density devices such as Intel Cyclone 10 GX, it may be necessary to wait up to 500 ms for the CvP status register bit assertion." According to  the Cyclone 10 GX devices achieve proper operation within 100 ms (via the PCIe IP core and CvP). I think (and here the documentation is a bit lacking), that this remark is valid only for other bits of the status register, e.g., CVP_CONFIG_DONE or USERMODE. I also think, that the 500 ms delay is calculated from peripheral + core image programming and that the time for peripheral image programming is far lower than that (i.e., low enough to allow PCI enumeration). But if this actually means that it can take up to 500 ms to program the peripheral image, than such FPGAs would have different problems. I.e., missing the deadline for PCI enumeration. This would need a solution outside of the scope of the altera_cvp module (e.g., soft-reset to re-start enumeration with a stable system). Bottom line: The CVP_EN should be deemed stable when altera_cvp is called, if not, the programming of the Intel/Altera FPGA and PCIe IP core has not been completed in time for the enumeration of the PCI device. Hence it would be questionable or, more likely, would not have completed successfully in the first place, i.e., altera_cvp would not have been called.  "Intel(r) Cyclone(r) 10 GX CvP Initialization over PCI Express User Guide", 2018.01.02  "Intel(r) Cyclone(r) 10 GX Device Overview", 2017.05.08 With best regards, Andreas
next prev parent reply other threads:[~2018-10-23 18:46 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-22 13:15 Andreas Puhm 2018-10-22 13:34 ` Eric Schwarz 2018-10-22 14:04 ` Moritz Fischer 2018-10-23 16:26 ` Anatolij Gustschin 2018-10-23 18:46 ` Andreas Puhm [this message] 2018-10-24 9:51 ` Moritz Fischer 2018-10-24 23:00 ` matthew.gerlach 2018-10-25 8:44 ` AW: " Andreas Puhm 2018-10-28 17:35 ` Moritz Fischer
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=3773264cdfcb4c258cc7eebd213302dd@SRV177.busymouse24.de \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: AW: [PATCH] fpga: altera_cvp: restrict registration to CvP enabled devices' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).