-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
opaec++: update doxygen for the current classes #140
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good 😀
Just a few comments and I'm wondering if we should be a little more generic and not refer to accelerators as much but instead refer to fpga resources. Thoughts?
common/include/opae/cxx/handle.h
Outdated
@@ -36,14 +36,26 @@ namespace opae { | |||
namespace fpga { | |||
namespace types { | |||
|
|||
/** An allocated accelerator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use accelerator
or something more generic like fpga resource
?
A handle
can represent either logical accelerator object or a physical fpga object.
In other words, can a fpgaconf be rewritten to use this C++ API (upper layer included) and if so, what would it look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I was purposefully staying away from fpga so that we didn't limit ourselves. How about "accelerator resource"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limit ourselves to just FPGA SW? The namespace already includes fpga...
common/include/opae/cxx/properties.h
Outdated
* | ||
* properties are information describing an | ||
* accelerator resource that is identified by | ||
* its token. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add something about properties also being used as a query for enumeration?
operator fpga_properties() const { return props_; } | ||
|
||
/** Retrieve the properties for a given token object. | ||
* @param[in] t A token identifying the accelerator resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment to a previous one. Should we use something more generic like fpga resource
?
common/include/opae/cxx/token.h
Outdated
@@ -37,16 +37,30 @@ namespace opae { | |||
namespace fpga { | |||
namespace types { | |||
|
|||
/** Wraps the OPAE fpga_token primitive. | |||
* token's are enumerated based on a set of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an apostrophe in tokens. What about changing this to read something like:
tokens are created from an enumeration operation that uses properties describing an fpga resource
common/include/opae/cxx/handle.h
Outdated
* | ||
* Represents an accelerator resource that has | ||
* been allocated by OPAE. The accelerator's | ||
* MMIO regions are made available for reading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. MMIO is implementation for register access. If you want to keep things generic, maybe you could rephrase this part:
Depending on the type of resource, its registers space may be read/written using a handle object.
We may also think about this too for the documentation for the read/write functions.
common/include/opae/cxx/handle.h
Outdated
@@ -80,9 +91,29 @@ class handle { | |||
*/ | |||
uint8_t *mmio_ptr(uint64_t offset) const { return mmio_base_ + offset; } | |||
|
|||
/** Allocate an accelerator, given a raw fpga_token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allocate or Open? Also, should we use "accelerator resource"?
* opae.admin: Add virtualization APIs * Cleanup sriov related attributes/methods * Move port assing/release into fme class * Fix region.ioctl method * remove mutate_flag from ioctl call * default mode to 'w' * tested/works on both Python 2.7 and Python 3.6 * refactor fpgaport * opae.admin: sriov: change code per suggestion
* opae.admin: Add virtualization APIs * Cleanup sriov related attributes/methods * Move port assing/release into fme class * Fix region.ioctl method * remove mutate_flag from ioctl call * default mode to 'w' * tested/works on both Python 2.7 and Python 3.6 * refactor fpgaport * opae.admin: sriov: change code per suggestion
a53e62e ofs-umd: fix ofs_add_driver cmake macro (#140) 59ec00f libofs: add libofs and umd (user mode driver) framework 9561585 libofs: fix blank lines in files b75690c libofs: finish migration to opae-libs repo f552e4d ofs_parse: look for call nodes in ast 049e00a ofs-umd: fix ofs_resolve to check func.id 4006206 libofs: add macros to wait for bit fields 15bac6c ofs umd: add ofs umd framework (#1909) e62592b ofs: initial implementation of libofs (#1899) git-subtree-dir: opae-libs git-subtree-split: a53e62e
Use OPAE_LIBS_ROOT when referencing dependancy ofs_parse.py Signed-off-by: Rodrigo Rojo <rodrigo.rojo@intel.com>
No description provided.