[casper] Suggested convention for writing registers

[James Smith]

Hello all,

I've given some thought to the topic of writing (and reading) registers on
the ROACH using the python corr module. Often in a design a single register
may be sliced into many bits to control various things. The way I've
normally seen such a register written in python looks something like this:

fpga.write_int('control',1<<9|1<<10|0<<25|1<<2|1<<3)

My feeling is that this approach is difficult to maintain - inheriting code
from someone else (or even from one's self 6 months down the line) is
likely to bring about some confusion in this case, and lead to a fair
amount of spelunking through the Simulink model in order to figure out what
bit 9 and bit 10 etc. do. On top of this, it places limitations on changing
one of the bits later without modifying the other ones - bitwise or
functions work well enough when you're over-writing zeros, but if there's
something there already it might not work so well.

With this in mind, I would like to suggest a convention which I worked out.
It uses python modules "struct" and "construct" to make code a bit easier
to read.
(For reference if anyone is unfamiliar:
struct - https://docs.python.org/2/library/struct.html
construct - http://construct.readthedocs.org/en/latest/ )

In the design I'm working on at the moment (a wideband spectrometer), I
wrote a python module with the following in it:

# Bitstruct to control the control register on the ROACH
control_reg_bitstruct = construct.BitStruct('control_reg',
    construct.Padding(4),                           #28-31
    construct.BitField('debug_snap_select',3),      #25-27
    construct.Padding(3),                           #22-24
    construct.Flag('fine_tvg_en'),                  #21
    construct.Flag('adc_tvg'),                      #20
    construct.Flag('fd_fs_tvg'),                    #19
    construct.Flag('packetiser_tvg'),               #18
    construct.Flag('ct_tvg'),                       #17
    construct.Flag('tvg_en'),                       #16
    construct.Padding(4),                           #12-15
    construct.Flag('fancy_en'),                     #11
    construct.Flag('adc_protect_disable'),          #10
    construct.Flag('gbe_enable'),                   #09
    construct.Flag('gbe_rst'),                      #08
    construct.Padding(4),                           #04-07
    construct.Flag('clr_status'),                   #03
    construct.Flag('arm'),                          #02
    construct.Flag('man_sync'),                     #01
    construct.Flag('sys_rst') )                     #00

This BitStruct makes the code a little bit more readable, tells you what
each bit does, and if you've done this declaration right once, then you
don't need to worry about whether you'e shifting numbers by the right
amount of bits. For the BitFields where several bits are passed, I used a
dictionary to make remembering things (and reading the code) easier as well:

# Dictionary for selecting the debug_snap_select bit
debug_snap_select = {
    'coarse_72':   0,
    'fine_128':    1,
    'quant_16':    2,
    'ct_64':       3,
    'xaui_128':    4,
    'gbetx0_128':  5,
    'buffer_72':   6,
    'finepfb_72':  7 }

So writing to the register for the first time works like this:

control_reg = avn.control_reg_bitstruct.parse('\x00\x00\x00\x00') # Create
a blank one to use...
    # Pulse arm and clr_status high, along with setting gbe_enable and
adc_protect_disable high
    control_reg.gbe_enable = True
    control_reg.adc_protect_disable = True
    control_reg.clr_status = True
    control_reg.arm = True
    fpga.write_int('control', struct.unpack('>I',
avn.control_reg_bitstruct.build(control_reg))[0]) # The [0] is necessary
because the fpga.write_int function wants an integer datatype, and
struct.unpack returns a tuple for some reason.
    # Bring arm and clr_status low again.
    control_reg.clr_status = False
    control_reg.arm = False
    fpga.write_int('control',
struct.unpack('>I',avn.control_reg_bitstruct.build(control_reg))[0])

Then, for example if you're controlling something with a function and you
need to change only one part of what's in the register and leave the rest
unaffected, this is quite easy as well:

    control_reg =
control_reg_bitstruct.parse(struct.pack('>I',fpga.read_uint('control'))) #
Read the data that's in the register already
    control_reg.debug_snap_select = debug_snap_select['coarse_72'] # Update
the desired bit (or bits, using the conveniently provided dictionary in
this case)
    fpga.write_int('control', struct.unpack('>I',
control_reg_bitstruct.build(control_reg))[0]) # Write the new data back.

Does anyone have any thoughts? Would this perhaps help with maintainability
and debugging, or is it a bit of overkill, solving a problem that wasn't
really there in the first place? I'm eager to hear any feedback from others
who may have walked this path before.

I've formatted the python code portions of the above message to a
fixed-width font, I hope it comes through on the mailing list...

Regards,
James Smith