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

New Bug in UHAL Address Tables? #39

Open
bdorney opened this issue Jul 1, 2019 · 11 comments
Open

New Bug in UHAL Address Tables? #39

bdorney opened this issue Jul 1, 2019 · 11 comments

Comments

@bdorney
Copy link

bdorney commented Jul 1, 2019

Seems like there are still a few bugs in the UHAL address table that where not mentioned in issue #37.

$ vfat_info_uhal.py --shelf=1 -s2 -g 11
01 Jul 2019 11:59:10.193 [7f163d870740] INFO - optohybrid_user_functions_uhal::getOHObject <> - gem.shelf01.amc02.optohybrid11: Success!
01 Jul 2019 11:59:10.193 [7f163d870740] INFO - optohybrid_user_functions_uhal::getOHObject <> - gem.shelf01.amc02.optohybrid11: Success!

--=======================================--

Firmware:     Version        Date
AMC     :       3.7.2   23/5/2017
Traceback (most recent call last):
  File "/opt/cmsgemos/bin/vfat_info_uhal.py", line 84, in <module>
    print "OH      :  %10s  %10s"%(getFirmwareVersion(ohboard,options.gtx),
  File "/usr/lib/python2.7/site-packages/gempython/tools/optohybrid_user_functions_uhal.py", line 86, in getFirmwareVersion
    fwver = readRegister(device,"%s.RELEASE.VERSION"%(baseNode),debug)
  File "/usr/lib/python2.7/site-packages/gempython/utils/registers_uhal.py", line 35, in readRegister
    device.getNode(register).getPath(),
uhal._core.exception: No branch found with ID-path "GEM_AMC.OH.OH11.FPGA.CONTROL.RELEASE.VERSION" from node "top"
Partial match "GEM_AMC.OH.OH11" found for ID-path "GEM_AMC.OH.OH11.FPGA.CONTROL.RELEASE.VERSION" from node "top"

@jsturdy points out that:

FPGA node is missing from uhal address tables

This was observed in FW release 3.8.3.

Not sure if this was resolved in 3.8.4 as well.

@evka85 ?

@jsturdy
Copy link
Contributor

jsturdy commented Jul 1, 2019

1698580 is a regression

commit 169858013b8195a508a271a803016041321777b1
Author: Evaldas Juska <evka85@gmail.com>
Date:   Wed Jun 12 21:27:14 2019 +0200

    v3.8.4  Added a possibility to switch to 40MHz promless programming mode; added VFAT-VFAT mixed BC and mixed EC flags in the data format; removed ADC monitoring from the SCA controller

diff --git a/scripts/address_table/gem_amc_top.xml b/scripts/address_table/gem_amc_top.xml
--- a/scripts/address_table/gem_amc_top.xml
+++ b/scripts/address_table/gem_amc_top.xml
@@ -1268,11 +1197,37 @@
     <node id="OH"  address="0x00400000"
           description="Optohybrid Registers"
           fw_is_module="true"
           fw_is_module_external="true">
 
       <node id="OH${OH_IDX}"  address="0x0"
             description="Optohybrid ${OH_IDX}"
             generate="true" generate_size="12" generate_address_step="0x00010000" generate_idx_var="OH_IDX">
 
-            <!--Insert here the OH FPGA module -->
-            <xi:include href="optohybrid_registers.xml"/>
+        <!--TTC module -->
+        <node id="CONTROL"  address="0x0000"
+            description="Implements various control and monitoring functions of the Optohybrid">
+
+            <node id="LOOPBACK" address="0x0"
+                description="Loopback Test Register" fw_signal="loopback">
+                <node id="DATA" address="0x0" permission="rw"
+                    description="Loopback data"
+                    fw_signal="loopback"
+                    fw_default="0x01234567"/>
+            </node>
+
+            <node id="RELEASE" address="0x1"
+                description="Optohybrid Status">
+                <node id="YEAR" address="0x1" permission="r"
+                    mask="0xffff0000"
+                    description="Release year"
+                    fw_signal="RELEASE_YEAR"/>
+                <node id="MONTH" address="0x1" permission="r"
+                    mask="0x0000ff00"
+                    description="Release month"
+                    fw_signal="RELEASE_MONTH"/>
+                <node id="DAY" address="0x1" permission="r"
+                    mask="0x000000ff"
+                    description="Release day"
+                    fw_signal="RELEASE_DAY"/>
+            </node>
+

@jsturdy
Copy link
Contributor

jsturdy commented Jul 1, 2019

Though I know we had discussed previously that this needs to be redone in a much better way, as currently the uhal address tables are generated for a fixed OH FW version...
The uhal XML parser supports the module node, so translating the xi:include to this in the generate_registers.py script seems to be a good solution.

@evka85
Copy link
Owner

evka85 commented Jul 1, 2019

Hi Jared, it seems that I have committed the old-style XML to the repo (without the include) as you pointed out, but I checked the XML files in the release zip file, and they are correct (the ones with xi:include).
Indeed the generated uhal XMLs included in the release contain registers from a fixed OH version. Are you saying that I should replace that with a module node? I can do that easily, but if you could provide an example for the module node, that would be nice.

@jsturdy
Copy link
Contributor

jsturdy commented Jul 2, 2019

The syntax that the uhal parser expects is (from here):

<node id="TOP">
  <node id="D1" module="file://mymodule.xml" address="0x00000400" />
  <node id="D2" module="file://mymodule.xml" address="0x00000500" />
</node>

So my suggestion would be to have the generated uhal address table look like this:

<node id="TOP">
...
  <node id="OH">
    <node id="OHXX" module="file://{GEM_ADDRESS_TABLE_PATH}/optohybrid_registers.xml" address="0xfeedcafe" /> <!-- The first node in this file should be the FPGA node (or maybe 'top') -->
  </node> <!-- End of OH block -->
...
</node>

@evka85
Copy link
Owner

evka85 commented Jul 2, 2019

Thanks, but actually the OHXX node also contains GEB, which is in the CTP7 firmware domain, so I still need to keep the OHXX node with the GEB stuff, and add an OH module inside... If Andrew can remove the FPGA node from his file, then I could have the FPGA node as a module..

Besides that, may I suggest that we just use the module node also in our parser instead of the xi:include? This would make things more consistent, with no find/replace needed, and best of all this would revive the parsing on CTP7 again (currently it's broken since the parser on CTP7 doesn't support the xi:include, so we have to resort to pickles only)!

@jsturdy
Copy link
Contributor

jsturdy commented Jul 4, 2019

Thanks, but actually the OHXX node also contains GEB, which is in the CTP7 firmware domain, so I still need to keep the OHXX node with the GEB stuff, and add an OH module inside... If Andrew can remove the FPGA node from his file, then I could have the FPGA node as a module..

Makes sense to me.

Besides that, may I suggest that we just use the module node also in our parser instead of the xi:include? This would make things more consistent, with no find/replace needed, and best of all this would revive the parsing on CTP7 again (currently it's broken since the parser on CTP7 doesn't support the xi:include, so we have to resort to pickles only)!

I can't comment on the feasibility of this, as I suspect @mexanick would have to adapt the parser he wrote (but using xi:include was a way to move away from the tradition of homewbrew solutions to solved problems)

@bdorney
Copy link
Author

bdorney commented Aug 20, 2019

New bug as of 3.9.0

amc_info_uhal.py --shelf=1 -s2
...
...
Traceback (most recent call last):
  File "/path/2/my/venv/cc7/py2.7/lib/python2.7/site-packages/gempython/scripts/amc_info_uhal.py", line 103, in <module>
    print "-> DAQ status reg     :0x%08x"%(readRegister(amc,"GEM_AMC.DAQ.STATUS"))
  File "/path/2/my//venv/cc7/py2.7/lib/python2.7/site-packages/gempython/utils/registers_uhal.py", line 35, in readRegister
    device.getNode(register).getPath(),
uhal._core.exception: No branch found with ID-path "GEM_AMC.DAQ.STATUS" from node "top"
Partial match "GEM_AMC.DAQ" found for ID-path "GEM_AMC.DAQ.STATUS" from node "top"

Looks like there was an unintended change in the uhal address table.

I've attached a diff of the address table for 3.8.6 and 3.9.0 and it seems GEM_AMC.DAQ.STATUS block has been completely removed.

Bit surprised by this, I was expecting the address space to have moved around not a complete drop in node names. If node names are removed this bricks the SW.

@bdorney
Copy link
Author

bdorney commented Aug 20, 2019

@evka85
Copy link
Owner

evka85 commented Aug 21, 2019

Hi Brian, sorry I forgot to regenerate the uhal XMLs after some experiments... I've fixed it and re-uploaded the address table zip file (only the uhal XML files are affected, nothing else changed). Sorry for overlooking it and causing trouble...

@jsturdy
Copy link
Contributor

jsturdy commented Nov 12, 2019

Coming back to this, because it's impacting development, we need the uhal address tables to be properly using the module attribute.

This requires a change in the OH xml (more realistically, an additional file to be prepared, as in cms-gem-daq-project/OptoHybridv3#23)

@jsturdy
Copy link
Contributor

jsturdy commented Nov 13, 2019

@evka85 additionally, the uhal address tables for 3.9.0 seem to be broken (it seems like they're broken for all recent releases, so perhaps I had always been making fixed versions while waiting for the fix to be merged in to the v3 trunk).
cf. #12 (comment)

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

3 participants