search

robertofem / CycloneVSoC-examples

Examples using the Cyclone V SoC chip
GNU General Public License v3.0
104 stars 40 forks source link

Minor defect in ghrd_top.v #6

Open affe00 opened 5 years ago

affe00 commented 5 years ago

Hello,

In CycloneVSoC-examples/FPGA-hardware/DE1-SoC/FPGA_DMA/ghrd_top.v, line 335:

.axi_signals_aruser (pio_controlled_axi_signals[ARPROT_BASE+: ARPROT_SIZE]), .axi_signals_arprot (pio_controlled_axi_signals[ARUSER_BASE+: ARUSER_SIZE])

The name of inputs and the parameter macros seems to be reversed.

This might not be a problem now, when you use them, it may be hard to understand.

Best Regards, affe00

robertofem commented 5 years ago

Hi,

thank you for mentioning the problem. Yes they are reversed. It is strange. They have different width and the compiler did not complain. Anyway I dont think these signals affect the performance. AXCACHE are the ones really important. I will add this as an issue into the repo to fix it when i have moretime. If you fix it and you want to do a pull request it is also welcomed.

Best regards, roberbot

El mar., 11 dic. 2018 a las 16:00, affe00 (notifications@github.com) escribió:

Hello,

In CycloneVSoC-examples/FPGA-hardware/DE1-SoC/FPGA_DMA/ghrd_top.v, line 335:

.axi_signals_aruser (pio_controlled_axi_signals[ARPROT_BASE+: ARPROT_SIZE]), .axi_signals_arprot (pio_controlled_axi_signals[ARUSER_BASE+: ARUSER_SIZE])

The name of inputs and the parameter macros seems to be reversed.

This might not be a problem now, when you use them, it may be hard to understand.

Best Regards, affe00

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/robertofem/CycloneVSoC-examples/issues/6, or mute the thread https://github.com/notifications/unsubscribe-auth/AOISzPZewvwMfUoYmwhayr84R5xyinkdks5u38hvgaJpZM4ZNlbF .

ranshalit commented 5 years ago

Hi, Is it fixed in git ? what should be the fix ? Thanks

robertofem commented 5 years ago

No. It is not fixed in git yet, otherwise I would have closed the issue. What should be changed is that ARPROT macros are asigned to aruser signals and viceversa. You have to switch the content inside the parenthesis in these 2 lines. You can do it and do a pull request if you wish