Skip to content
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

Refactor sysfs usage in HAL #104

Closed
rradjabi opened this issue Jul 18, 2018 · 7 comments
Closed

Refactor sysfs usage in HAL #104

rradjabi opened this issue Jul 18, 2018 · 7 comments
Assignees

Comments

@rradjabi
Copy link
Contributor

Lots of code duplication exists in HAL for accessing sysfs. As such, we have many instances of slight variations of the same functions. This can cause issues in the future. It would be best to have a sysfs access class or something like that.

Examples:
flasher.cpp L166,175
memaccess.h L62
debug.cpp L137
hwmon.h L38
scan.h L24
shim.cpp L637,749,1362,1366,1388
xbsak.h L197,318,537
xbsak_debug.cpp L42,236
appdebug.cpp L1029

@stsoe
Copy link
Collaborator

stsoe commented Jul 19, 2018

I agree, this looks like some dangerous implicit assumptions about sysfs. Absolutely should be factored into one access point of some sort.

@maxzhen maxzhen self-assigned this Jul 19, 2018
@maxzhen
Copy link
Collaborator

maxzhen commented Jul 19, 2018

I'll clean it up.
What is the danger you're seeing here other than duplicate code?

@rradjabi
Copy link
Contributor Author

rradjabi commented Jul 19, 2018 via email

@maxzhen maxzhen removed their assignment Jul 19, 2018
@maxzhen
Copy link
Collaborator

maxzhen commented Jul 19, 2018

Oops, I just removed myself from the assignee. Maybe all of us, in this team, should have write permission?

@sonals
Copy link
Member

sonals commented Jul 19, 2018

Please use C++ templatized methods to modularize this code. The same templatized function should be used to read data types of different types and will be type safe.

rradjabi added a commit to rradjabi/XRT that referenced this issue Jul 20, 2018
* Experiment with templates to open sysfs/.../ip_layout and /
mem_topology. This is complicated and currently doesn't compile.
* The function for getting xclbinid works.
* Refs Xilinx#104
@hcneema
Copy link
Contributor

hcneema commented Sep 7, 2018

post 2018.2_XDF

@maxzhen
Copy link
Collaborator

maxzhen commented Oct 2, 2018

Implemented in PR#399.

@maxzhen maxzhen closed this as completed Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants