Morning James

We do this in already in the casperfpga package:
https://github.com/ska-sa/casperfpga

It turns out construct is pretty slow, so we just do the conversions and
shifts manually. I think currently the fpg files that are automatically
parsed by casperfpga are only generated for ROACH2, but it would be easy to
add ROACH support.

Cheers
Paul




On 20 March 2015 at 09:08, James Smith <jsm...@ska.ac.za> wrote:

> 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
>
>
>

Reply via email to