evka85 / GEM_AMC

GEM uTCA AMC common firmware logic as well as board specific implementations for GLIB (Virtex 6) and CTP7 (Virtex 7)
0 stars 11 forks source link

New Bug in UHAL Address Tables? #39

Open bdorney opened 5 years ago

bdorney commented 5 years ago

Seems like there are still a few bugs in the UHAL address table that where not mentioned in issue https://github.com/evka85/GEM_AMC/issues/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 commented 5 years ago

16985801 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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@jsturdy @evka85 @mexanick comment here

evka85 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

@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. https://github.com/evka85/GEM_AMC/pull/12#issuecomment-553357638