Added basic SYSMON interfaces for both UltraScale and UltraScale+ FPG…

[jhegeman]

NOTE: Four different dep files are available: two each for US and USP, covering the use cases with and without external 1.25 V reference source. (Naming should be self-explanatory.)

The UltraScale+ versions have been tested. The UltraScale versions are creative copy-paste based on Xilinx UG580, but I don't have an UltraScale FPGA to truly test with.

[jhegeman]

Ok, after our email conversation I have updated my branch to reflect the naming convention that IPbus slaves have an 'ipbus_' prefix.

[tswilliams]

Hi,

Thanks for updating the code to match the naming convention discussed over email.

Alessandro and I have taken a look at the changes, and we have a few comments/requests:

• To avoid proliferation of example designs in pull request #152, it would make sense to us to include the DNA and SYSMON slaves in the same example design (rather than have two separate ones). With that in mind, in some of the requests below, in some filenames I suggest changing sysmon to xilinx (though if you think there's a much more appropriate collective term for the SYSMON and DNA slaves, do let us know)

• For consistency with the locations of the XML & VHDL files for the existing example designs, could you move:

  • projects/example/addr_table/example_sysmon_{usp,x7}.xml to components/utils/addr_table/ipbus_example_xilinx_{usp,x7}.xml
  • projects/example/firmware/cfg/example_sysmon_{usp,x7}.dep to components/utils/firmware/cfg/ipbus_example_xilinx_{usp,x7}.dep
  • projects/example/firmware/hdl/example_sysmon_{usp,x7}.vhd to components/utils/firmware/hdl/payload_xilinx_{usp,x7}.vhd

• To distinguish the files for the two types of examples more clearly, can you create a new directory - projects/xilinx - and move:

  • projects/example/firmware/cfg/top_{kc705_gmii,vcu118_sgmii}_sysmon.dep to projects/xilinx/firmware/cfg/top_{kc705_gmii,vcu118_sgmii}.dep
  • projects/example/firmware/scripts/example_sysmon.py to projects/xilinx/scripts/example_sysmon.py
  • projects/example/firmware/scripts/README to projects/xilinx/README.md (and put the lists of commands in Markdown comment blocks)

Cheers, Tom

[jhegeman]

Hi Tom,

I can do this, indeed. I'm not sure about the comment about the proliferation of example designs, but that's your choice to make. (Personally I prefer minimal examples demonstrating a single feature.)

I'd suggest a different approach though. By now we have collected several related pull requests. How about you merge #145 , #151 , and #152. Then we have all code in the same branch again. I can then update my fork from your master and make the changes, followed by a pull request with all changes in one go. (That would also allow to apply the necessary changes to the sysmon after changing the DRP address width.)

Cheers, Jeroen

[tswilliams]

Hi,

I'd suggest a different approach though. By now we have collected several related pull requests. How about you merge #145 , #151 , and #152. Then we have all code in the same branch again. I can then update my fork from your master and make the changes, followed by a pull request with all changes in one go. (That would also allow to apply the necessary changes to the sysmon after changing the DRP address width.)

I'm not sure I understand the reasoning behind this different approach. In order for a pull request to be merge-able, if missing any of the target branch's recent the pull request's branch needs to be rebased on top the target branch (in this case master), or otherwise have all the target branch's commits merged into them.

As a result, if this pull request is the first of the three to be merged in, then the branch for pull request #152 will need to be rebased on top of the master branch after this pull request has been merged in. So, the branch for pull request #152 would include the changes from this pull request before it is merged. Similarly the pull request for #153 would need to be rebased (and hence include the changes from this pull request and #152) before being merged, meaning that DRP-address-width-related changes for the SYSMON slaves could be included in pull request #153

Cheers, Tom

[jhegeman]

Okay.... I will stick everything together then. But please try to be responsive in merging pull requests. That would avoid a lot of the rebasing, rebuilding, and retesting.

[jhegeman]

Ok, done. Please try this version.

[tswilliams]

Hi Jeroen,

Thanks for updating the code, it's greatly appreciated.

I've added a couple of commits with minor changes (e.g. licence headers comments in VHDL files), and pushed them to this repo. But to be included in this PR they need to be transferred over to your fork, so can you update your branch for this pull request to point to commit c98b3cb8b2b3850fdea7c1e2d2f9966dee58d56b? You can do this as follows:

git remote add github-ipbus https://github.com/ipbus/ipbus-firmware.git
git fetch github-ipbus

# Assuming the 'remote' for your fork of the IPbus repo is named "origin" (if not replace origin in following command with whatever name you used - that name can be found by running "git remote -v")
git push -f origin c98b3cb8b2b3850fdea7c1e2d2f9966dee58d56b:ipbus_issue144

I'll then merge this branch in, and we can move onto the next PR :)

Cheers, Tom

[jhegeman]

Ok, done. I'm not sure what state my work area is in now. It looks rather confused so let's hope the PR works now ;-)

Jeroen

[tswilliams]

run tests, please

[tswilliams]

Hi,

Branch has been merged. Small request: When updating your branch for pull request #152, can you rebase the branch on top of master, rather than using git merge? (Motivation: The commit history is simpler after the rebase.) I.e:

git fetch github-ipbus

git checkout ipbus_issue151
git rebase github-ipbus/master
git push -f origin ipbus_issue151

Cheers, Tom