What is a better write enable

[pcprogrammer]

I'm making a micro controller interface with a FPGA and like to make it synchronous with the internal FPGA clock to be able to write different registers within the design.

Found synchronization code with a shift register and that works in the simulator where it shows a one internal clock period long pulse on the intended edge of the external pulse. When I follow BrianHG his advise to basically always use the posedge of the main clock the pulse starts on the rising edge of this clock and also ends on the rising edge. Is this good for enabling the write of a register or is it better when the pulse starts and ends on the falling edge of the main clock, having the enable active for sure on the rising edge of the clock.

Code for the pulse starting and ending on the rising edge

Code: [Select]

  //---------------------------------------------------------------------------
  //Synchronize to fpga clock system

  reg mcu_clk0 = 0;
  reg mcu_clk1 = 0;
  reg mcu_clk2 = 0;
  reg mcu_enable = 0;

  //Sample the mcu clock input on the up going edge of the main clock
  always@(posedge i_main_clk)
    begin
      mcu_clk2 <= mcu_clk1;
      mcu_clk1 <= mcu_clk0;
      mcu_clk0 <= i_clk;

      mcu_enable <= !mcu_clk1 & (mcu_clk2 ^ mcu_clk1);  //Only enable on the falling edge of the external clock
    end

Code for the pulse starting and ending on the falling edge

Code: [Select]

  //---------------------------------------------------------------------------
  //Synchronize to fpga clock system

  reg mcu_clk0 = 0;
  reg mcu_clk1 = 0;
  reg mcu_clk2 = 0;
  reg mcu_enable = 0;

  //Sample the mcu clock input on the up going edge of the main clock
  always@(negedge i_main_clk)
    begin
      mcu_clk2 <= mcu_clk1;
      mcu_clk1 <= mcu_clk0;
      mcu_clk0 <= i_clk;

      mcu_enable <= !mcu_clk1 & (mcu_clk2 ^ mcu_clk1);  //Only enable on the falling edge of the external clock
    end

The only difference is the edge of the clock being used to make the pulse, which for the second is against the advice of BrianHG. When the second is better timing wise but bad for clock performance is there a better way of coding it to get the same result?

The first screen capture shows the pulse starting and ending on the rising edge. The second one shows it starting and ending on the falling edge.

[BrianHG]

It depends on how your MCU output drives the FPGA.

Are you using a bit-bang approach to driving the MCU IOs?
Are you using an MCU's dedicated UART IO, or SPI/I2C port?

If your MCU UART is fast, like if it can achieve megabaud rates, it would actually be easiest to just use my Verilog UART and snap a string command line.  (This also makes interfacing with things like RPI as their library already contains HW UART commands.)  If you only need a single megabaud, again using a UART interface means just hooking the same IO and code to a USB-RS232 adapter and you can command your unit from a PC or MCU alike as well as have a status return channel.  A simple 1 wire, or 2 wire for bidirectional interface.

[pcprogrammer]

The MCU interface is bit banged IO. It is an Allwinner F1C100s. Have not measured the write pulse yet, but had some code working that was made for the FNIRSI 1013D scope. The problem with that is that the register writing is not done with the main clock, but with a to the main clock synchronized clock, which I like to change to this write enable setup.

Edit: It is an 8 bit parallel  interface with a read/write signal and a data/command signal.

[BrianHG]

An easy bitbang approach is the same as my other thread's:

Code: [Select]

always @(posedge clk_125) begin
clk_half <= !clk_half ; // Generate a 62.5MHz clock in phase with the 125MHz dac data.
endwhich is already super close to your current setup.

I'm assuming you want a parallel data buss input with multiple words for super speed, plus the enable.

example (Excuse my HDL shorthand):

Code: [Select]

input         clk_125mhz,
input         data_clock,
input         data_packet_ready,
input [7:0] data_in_bus,

output  reg [xx:0] ctl_1,
output  reg [xx:0] ctl_2

reg [7:0] cmd_packet [0:2];  // A store of a string of bytes which will be our command.

reg prev_data_clock = 0;

always @(posedge clk_125mhz) begin

prev_data_clock <=  data_clock ;

  if (prev_data_clock != data_clock ) begin // D-Latch and shift hold the last 3 transmitted bytes every time the data_clock  toggles
      cmd_packet [0] <= data_in_bus ;
      cmd_packet [1] <= cmd_packet [0] ;
      cmd_packet [2] <= cmd_packet [1] ;
  end

  if (data_packet_ready) begin // when the data packet is ready, check if the command drives any controls and latch them through in a clean single shot.
     if (cmd_packet [0]==8'h01) begin
                                   ctl_1[7:0]  <= cmd_packet[1];
                                   ctl_1[15:8] <= cmd_packet[2];
                                             end
     if (cmd_packet [0]==8'h02) begin
                                   ctl_2[7:0]  <= cmd_packet[1];
                                   ctl_2[15:8] <= cmd_packet[2];
                                             end

  end

end

Is this something like what you want to do?

[BrianHG]

When bitbanging for the above code, just:

Make sure the 'data_packet_ready' is clear.
TX Byte 1 on the 'data_in_bus'.
Invert the 'data_clock'
TX Byte 2 on the 'data_in_bus'.
Invert the 'data_clock'
TX Byte 3 on the 'data_in_bus'.
Invert the 'data_clock'
Set the 'data_packet_ready'.
Clear the 'data_packet_ready'.

So long as your MCU doesn't exceed 75MHz on the data transmision, you should capture clean data.

Note that you can expand the 'reg [7:0] cmd_packet [0:2]' size to 5 bytes, or make it 16bits wide depending on how you transmit your data bus.

You can even shrink the cmd_packet to 2 and have an 8 bit address and 8 bit data bus.

[BrianHG]

Sorry, didn't take into account the 'read back' function.
And if you only need an 8bit address with 1 8 bit data going at a time, you can further simplify my code and use 1 wire to transmit data and 1 wire to select read instead of write.

[BrianHG]

Example:

Code: [Select]

input         clk_125mhz,
input         data_clock,
input         data_readback,
inout [7:0] data_bus,

input         [7:0] rctl_1,
input         [7:0] rctl_2,
output  reg [7:0] wctl_1,
output  reg [7:0] wctl_2

reg [7:0] address;  // A store of a string of bytes which will be our command.
reg oe_data = 0;
red [7:0[ data_out = 0;

assign data_bus = oe_data ? data_out : 8'bzzzzzzzz ; // Generate the bi-directional 8bit IO port.

reg prev_data_clock = 0;

always @(posedge clk_125mhz) begin

prev_data_clock <=  data_clock ;

  if (!prev_data_clock && data_clock ) begin // Load address on the detected rising transition of the data_clock.
      address <= data_in_bus ;
  end

  if (prev_data_clock && !data_clock ) begin // Load data on the detected falling transition of the data_clock.
       if (address = 8'h01) wctl_1<=data_bus;
       if (address = 8'h02) wctl_2<=data_bus;
  end

// readback control

if (data_readback && data_clock ) begin // When the data_readback is high and only while the data_clock is kept high.

   oe_data <=1 ; // output enable 8 bit bus.

       if (address = 8'h81) data_out <= rctl_1 ;
       if (address = 8'h82) data_out <= rctl_2 ;


end else  oe_data <=0 ; // input enable 8 bit bus.

end

This code is missing a 'reset' at the beginning and you may add a check for output enable by looking at the stored high address bits to help protect your IOs.

To transmit data:

Make sure the 'data_clock' & 'data_readback' are low.
output enable the MCU 8bit data port
TX Address on the 'data_bus'.
Make 'data_clock' high
TX Control byte on the 'data_bus'.
Make 'data_clock' low.
Done. 

To read data:
Make sure the 'data_clock' & 'data_readback' are low.
output enable the MCU 8bit data port
TX Address on the 'data_bus'.
Make 'data_clock' high
Switch your MCU 8-bit bus to input mode and then make 'data_readback' high.
wait 1 nop
read 8 bit data bus.  (You can pause here, or continuously read here if you want to monitor a time sensitive status flag)
Make the 'data_clock' & 'data_readback' low.
Done. 

[BrianHG]

Now, if you were using SystemVerilog instead of Verilog, we could have made the 'wctl' out an 8 bit array with 256 words where any of your other modules could tap a byte or multiple bytes within.

Same for the read bus.


Example change:

Code: [Select]

if (prev_data_clock && !data_clock ) begin // Load data on the detected falling transition of the data_clock.
       if (address = 8'h01) wctl_1<=data_bus;
       if (address = 8'h02) wctl_2<=data_bus;
  end

Would change to:

Code: [Select]

if (prev_data_clock && !data_clock ) begin // Load data on the detected falling transition of the data_clock.
       wctl[address]<=data_bus;
  end

and at the top, a single:

output  logic [7:0] wctl [0:255]

[pcprogrammer]

Wow BrianHG you have been busy Thank you for all the input.

Lots of stuff to look into, and novel idea's on how to do a thing like this.

The software side of the FNIRSI uses a single byte command written to the FPGA first and then dependent on the command 1 to 4 data bytes can either be written or read. For the sample data it allows the successively reading of the samples after the command for it has been written. Forum member morris6 made a new implementation for the FPGA of the scope, but the reversed code would need several modifications for working with it and I did not had the drive anymore to continue with that project.

So I moved on to this AWG project. The idea is to be able to control two channels with a separate step setting for both the negative and the positive portion of the signal giving the ability to do "pulse width" modulation on every type of signal. This means 4x 48bits of step data. Also setting the phase needs to be able, so another 2x 48bits. An option to reset the channels phase registers, which can just be a command without data.

Especially the writing of the phase data needs to be synchronized with the 125MHz clock since it has to be loaded into the actual signal_phase registers.

I have to play with your ideas to see how that can work.

The project will end up in my lichee nano repository. The hardware portion is already there: https://github.com/pecostm32/Lichee_Nano

[BrianHG]

Setup all your regs with my second code.
Once done, make a single address which will allow you to snap the block of regs all at once.

IE: in system verilog:

Code: [Select]

if (wctl[0][127] && any_other_event_you_like)  all_at_once_wctl[0:255] <= wctl[0:255] ;

and add at the top:

output  logic [7:0] wctl [0:255],
output  logic [7:0] all_at_once_wctl [0:255]

Now, just make sure address 127 = 0.
Set all your regs.  This will progressively fill all of wctl[0:90] bytes or so...
Now, set address 127 = 1.  Now, in a single 125mhz clock, all of the 'all_at_once_wctl' will simultaneously change to the slowly filled 'wctl'.

You can also make a separate input called ' any_other_event_you_like' from the MCU or from counter and timers from your synthesizer to do the same thing.

[BrianHG]

My first example code was better suited to be used as an auto-address or multi-byte command with 1 address.

My second code is a direct addressing for each byte.

Depending on the instruction set of your MCU, method #1 might save 1-2 additional clock cycles per byte in a multi-byte command.  So, sending 4 bytes may save 3-6 MCU clocks VS my simpler version #2 where you need to stuff an address before each byte.

[pcprogrammer]

example (Excuse my HDL shorthand):

Code: [Select]

input         clk_125mhz,
input         data_clock,
input         data_packet_ready,
input [7:0] data_in_bus,

output  reg [xx:0] ctl_1,
output  reg [xx:0] ctl_2

reg [7:0] cmd_packet [0:2];  // A store of a string of bytes which will be our command.

reg prev_data_clock = 0;

always @(posedge clk_125mhz) begin

prev_data_clock <=  data_clock ;

  if (prev_data_clock != data_clock ) begin // D-Latch and shift hold the last 3 transmitted bytes every time the data_clock  toggles
      cmd_packet [0] <= data_in_bus ;
      cmd_packet [1] <= cmd_packet [0] ;
      cmd_packet [2] <= cmd_packet [1] ;
  end

  if (data_packet_ready) begin // when the data packet is ready, check if the command drives any controls and latch them through in a clean single shot.
     if (cmd_packet [0]==8'h01) begin
                                   ctl_1[7:0]  <= cmd_packet[1];
                                   ctl_1[15:8] <= cmd_packet[2];
                                             end
     if (cmd_packet [0]==8'h02) begin
                                   ctl_2[7:0]  <= cmd_packet[1];
                                   ctl_2[15:8] <= cmd_packet[2];
                                             end

  end

end


As a programmer the construct above feels counter intuitive I have to get used to another way of thinking, more parallel and less sequential.

The "prev_data_clock <=  data_clock;" before the "if(prev_data_clock != data_clock)" looks impossible. Assigning the signals to become the same and at the same time check that they are not

I do like the concept of having the MCU tell the FPGA that a transfer is done. This way there is no need for a counter to keep track of the bytes coming from the MCU and it gives a clean point in time for the FPGA to transfer the data to the intended register.

Also have to look at verilog modules differently then how I would build hardware with TTL logic. I was thinking of a data/address bus setup with control signals between the modules to transfer the data to the registers, instead of lots of wires between the modules. But since it is all compiled it does not matter if there is a big wire list between the main and the sub modules.

My way back experience with FPGA was making schematics in Orcad and then use XACT to turn them into a bitstream. Way different then with HDL.

[BrianHG]

Also have to look at verilog modules differently then how I would build hardware with TTL logic.

Note that what's illustrated here is still like TTL logic gates.

Think of each 'cmd_packet' as a 74LS574, data in from MCU to the first one, then data out of that one to the next 74LS574.

All 3 CLK ins of the 8bit parallel D-Flipflops 74LS574 are all tied to this logic:

(clk_125m && (prev_data_clock != data_clock))

This will pulse true when the 125m goes high and you input 'data_clock' just switched high or low compared to what it's value was 1 clk_125m ago.

Think of the cmd_packet in this example as a command line buffer and once the 'data_packet_ready' goes high, we look at the final cmd_packet[0] and decide what to do with the contents of the contents of the parallel byte shift in buffer snapping everything in a single 125MHz clock.

My code here minimizes toggles on your MCU's output port and operates so that you set the MCU data, then the data_clock guaranteeing that the data going into the FPGA is ready and clean before the capture takes place as long as your MCU isn't toggling the port above 125MHz.  Well, I guess you can run this section on the 250MHz clock if you like.  You would have a damn fast capable 8bit IO port.

[BrianHG]

Also, remember, if you used huge 64bit x 128 arrays called example:

Code: [Select]

output reg [63:0] settings [0:127]
using system verilog allowing array ports feature,  you should be able to tie to your other modules the single array address you like as your settings making everything a single net name with a single address.

Any unused signal wires in the array net is automatically removed/pruned by the FPGA compiler during compile time, so those resources never tied to anything wont ever be used as logic cells in the FPGA.

[pcprogrammer]

I know it is still like TTL logic and understand the concept. I was just pointing out that there is a difference in how things are done in the different languages. (Seeing TTL as a language)

Making a thing like this with TTL in schematic form would be easiest with an 8 bit data bus and some control signals. In verilog the connections between the modules could be done in the same way, but I think it is more complicated then just listing the needed registers in the module signal list, and handle all the transfers in the MCU interface module.

The MCU won't reach that high speeds. At max I think 2MB/s or so. SPI would be faster. I think it can reach 100Mb/s but doubt it will be reliable at that speed.

I do have another issue. I went from a flat design to a modular design with separate files for the different parts of the system. For the MCU interface this means having an inout [7:0] io_data signal for the connection with the IO pins. In the main module this is also in the signal list as a wire. (inout wire [7:0] io_mcu_d) In the main module these are connected to each other in the declaration of the MCU interface module. (    .io_data (io_mcu_d),)

On compile the system failed with an error that it could not maintain the Hiz this way and that the hierarchy should be flat. I added (*keep_hierarchy = "no"*) to the MCU interface module and that fixed it.

Is there another way of doing this?

[pcprogrammer]

Also, remember, if you used huge 64bit x 128 arrays called example:

Code: [Select]

output reg [63:0] settings [0:127]
using system verilog allowing array ports feature,  you should be able to tie to your other modules the single array address you like as your settings making everything a single net name with a single address.

Any unused signal wires in the array net is automatically removed/pruned by the FPGA compiler during compile time, so those resources never tied to anything wont ever be used as logic cells in the FPGA.

Since the whole HDL stuff is new to me, I like to focus on getting a grasp on standard verilog first.

Not sure about what the implication of the system verilog setup is on the MCU side. Is it still possible to write just data for a single register (single or multi byte) and activate that with a last write or do all the bytes have to be written every time?

[BrianHG]

Hmm, in Quartus,  I have the top-hierarchy where everything is connected to the io pins:

inout [7:0] io_data,

in my sub-modules, I have the same at every level.

I have never used:

inout wire [7:0] xxx_wire,

I would just place the:

.io_data (io_data),

when initiating the sub module.  The compiler knows that the . in front of the same net name means it's a port name inside the sub module while using the same of a different net should connect the 2.

Unless you wrote something different.

[BrianHG]

SystemVerilog is backwards compatible with Verilog.
You just save your files with a .sv instead of a .v.

System verilog just expands the lexicon and features.
It has a better handle when using negative / signed numbers.
It can better handle regs with arrays, or memories.
It has an improved 'combinational' logic capabilities.  (Immediate static logic, no clock)

It has a lot more simulation test bench functions.

[pcprogrammer]

Hmm, in Quartus,  I have the top-hierarchy where everything is connected to the io pins:

inout [7:0] io_data,

in my sub-modules, I have the same at every level.

I have never used:

inout wire [7:0] xxx_wire,

I would just place the:

.io_data (io_data),

when initiating the sub module.  The compiler knows that the . in front of the same net name means it's a port name inside the sub module while using the same of a different net should connect the 2.

Unless you wrote something different.

Strange thing in the Tang IDE then.

It does compile but I get this warnings.

Code: [Select]

SYN-5045 WARNING: Netlist check -- multi-inout net has non-mpin inout pin mcu.io_data[7].
SYN-5036 WARNING: Tri-buffer 'u2' can't be implemented by pad. Please flatten model 'mcu_interface' by adding directive 'synthesis keep_hierarchy'.

Did not test the result in the actual hardware.

Does not make a difference if there is wire in the main module signal list or not.

Probably not a big deal and it is fixed with the (*keep_hierarchy = "no"*) directive. Was just wondering about it.

[BrianHG]

On compile the system failed with an error that it could not maintain the Hiz this way and that the hierarchy should be flat. I added (*keep_hierarchy = "no"*) to the MCU interface module and that fixed it.

Is there another way of doing this?

Perhaps this is not a coding problem with the inout and inout wire, maybe the way you set your hierarchy order in your FPGA compiler setup is incorrect.

The module with the connections to the IO pins should be set to the 'top hierarchy'.
Inside that HDL module, you should be initiating all you sub modules, including the MCU interface module, and wiring them to the IO pins as well as together.  These modules should be below that top module.  If you have the order wrong, then yes, there may be a problem with you 8bit MCU interface trying to drive a tristate 8'bzzzzzzzz into a module beneath it instead of above.

[pcprogrammer]

By the looks of the project navigation the hierarchy should be in order.

The compiler just wants it to be flat to get it correct.

There is an actual difference in the number of configuration bits used, so the hardware will be different.

[BrianHG]

It does compile but I get this warnings.

Code: [Select]

SYN-5045 WARNING: Netlist check -- multi-inout net has non-mpin inout pin mcu.io_data[7].
SYN-5036 WARNING: Tri-buffer 'u2' can't be implemented by pad. Please flatten model 'mcu_interface' by adding directive 'synthesis keep_hierarchy'.

Did not test the result in the actual hardware.

Does not make a difference if there is wire in the main module signal list or not.

Probably not a big deal and it is fixed with the (*keep_hierarchy = "no"*) directive. Was just wondering about it.

Adding the directive 'synthesis keep_hierarchy' to the MCU module should then do it.

I'm not sure, but using that directive tells the compiler to follow / generate the hierarchy order the way in which you wrote the code instead of the way you have the modules listed with the top module and it's siblings in the compiler setup.

At least, in Quartus and Lattice Diamond, we tell the compile in it's settings menu which source file is the top hierarchy and which source files are it's children.  Otherwise, those compilers will also get confused.

[BrianHG]

By the looks of the project navigation the hierarchy should be in order.

The compiler just wants it to be flat to get it correct.

There is an actual difference in the number of configuration bits used, so the hardware will be different.

Wrong code, I need to see the wiring in you top hierarchy.

[pcprogrammer]

I tried using that directive 'synthesis keep_hierarchy' but it did not work. Google search gave me the (*keep_hierarchy = "xxx"*). Tried it with yes and no dice, but with no it solved it.

Have to look into if there are compiler settings to make things behave differently.

[pcprogrammer]

By the looks of the project navigation the hierarchy should be in order.

The compiler just wants it to be flat to get it correct.

There is an actual difference in the number of configuration bits used, so the hardware will be different.

Wrong code, I need to see the wiring in you top hierarchy.

Attached it the whole project.

[BrianHG]

Im curious, if you change:

  inout wire [7:0] io_mcu_d,

to:

  inout  [7:0] io_mcu_d,

it wont work?
Maybe someone else here knows why you are getting this problem.
If you really want, you can move the tristate gate outside the MCU module into your top module.  But that just adds a separate in and out ports plus an output enable port.

[pcprogrammer]

No it did not do the trick. Also tried using the same name on all levels. Did not do the trick.

I did find this settings menu, where it has the keep_hierarchy on auto. It allows flatten, auto, manual and all.

Setting it to flatten removes the warnings, but the number of used configuration bits also went down, so it changed the logic in some way.

Edit: Tried the bit stream on the hardware and it works as before, so the flatten option also does the trick.

[BrianHG]

No id did not do the trick. Also tried using the same name on all levels. Did not do the trick.

I did find this settings menu, where it has the keep_hierarchy on auto. It allows flatten, auto, manual and all.

Setting it to flatten removes the warnings, but the number of used configuration bits also went down, so it changed the logic in some way.

It probably merged some duplicate gates across multiple modules.

Also, in your MCU code, with your method, you are clocking logic using multiple clocks.
This is ok, but slows down FPGA fabric since the FPGA has a limited dedicated global clock array.

The way I wrote my interface, I'm clocking all the logic cells with the single 125MHz clock.
What is happening using my 'IF() ...', this causes the compiler to use the 'CE' or 'LE' on the D-flipflop logic cells.  So the FPGA fabric gates which runs the clock-enable/latch-enable are nets with the fabric designed for that purpose.  With your current code, the compiler needs to operate with multiple clock domains, 1 for each different @(posedge & negedge) net name and wire a special new net on the FPGA fabric for that purpose.  This is known as a bad practice as once that logic is clocked at a different time that the dedicated PLL clock line specifically timed to every logic cell on the FPGA, when feeding data from that custom home-made clock to the main system clock, you will be left with metastability issues.

To help fix that up, you will need a complex .sdc timing file describing each of your custom home-made generated clocks and how they relate to each other and how fast each one can go.

[pcprogrammer]

And that is why I started this thread

What I noticed in your solution is that there is no shift register for synchronizing the external signal. I read here https://www.doulos.com/knowhow/fpga/synchronization-and-edge-detection/ in the paragraph "So how do I do that edge detection?" that there is a possibility of instability on the first flip flop, so I used their setup with the three flip flops to do the synchronization code I started this thread with.

You just use the one register and then the "if" they are different to get it in sync with the 125MHz clock. Is there no risk with metastability there?

As per your advise I'm trying to move to an almost single clock design. Still need the extra 250MHz for the DAC clock and write signal.

[BrianHG]

Ok, let's look at it this way.
Twice, draw out on paper just 2 D-flipflop latches and how their clock are wired with simple gates.

#1, your example clock setup with your project;s net names using a separate gates to generate each clock to your final system 125MHz generator.

#2, same story, but tie all the clocks to the 125Mhz and use my 'logic enable' input to tell the D-flipflop whether it should go or not go.

Then I will show you how method #1 skews the output of each clocked flipflop data bus since the fpga's wired logic (which will now change every time you modify your code meaning a different delay) will add a random delay to those outputs.

Once you worked that one out, if you want to read this, you may help understand why 1 single clock design works simpler and how writing things differently may affect how the compiler wires the FPGA:
https://www.eevblog.com/forum/fpga/fpga-vga-controller-for-8-bit-computer/msg2767312/#msg2767312

[pcprogrammer]

A lot of this stuff is known to me, a bit rusty but known. Started "life" with hardware before I turned to software

I understand that having a single clock to all the flipflops is best practice. Also know a FPGA has dedicated clock lines with buffers throughout the chip and that they are limited.

As a "modern" FPGA and verilog learning step I made an asynchronous 14 bit counter to drive the DAC. I could see the difference on the logic analyzer where the different bits would be delayed from each other. Could also tell which bits were placed in the same slice, because they had minimal delay between them. (Tried reverse engineering the original programming of the FNIRSI 1013D FPGA, but failed on the routing between the blocks. I can tell how the IO is configured though and with a bit more effort also how the logic blocks are configured, but the connection matrix eludes me)

The VGA controller bit is very interesting and from a hardware perspective I would also have used version 3.

But this does not answer my last question about metastabillity. In your code the external signal is latched on the 125MHz clock and at the same time the compare is done to detect the edge. But what if at the moment of the rising edge of the clock the external signal is in transition? Could this cause problems like mentioned in the article. https://www.doulos.com/knowhow/fpga/synchronization-and-edge-detection/

[BrianHG]

Just draw the #1 and #2 schematic illustrations and post them here...

[pcprogrammer]

Ok here they are. Did not do the whole design since we are looking at the MCU interface.

Added a third one with the schematic of the code I started this thread with.

Number 1 is the MCU interface as it is now, and it is working, but I know morris6 has had weird problems with his design for the scope, which I copied this code from.
Number 2 is it modified to the 125MHz clock with the use of clock enables on the flip-flops.
Number 3 is just the external signal synchronization part.

The number 2 setup will take in the data on both the falling and the rising edge of the external clock pulse, for which you pointed out in your design that the MCU only needs to toggle instead of pulsing this line for each write.

The output of the data_x flip-flops is used directly by the signal_phase counter which is clocked on the 125MHz clock. The i_xtal signal in the first setup is 50MHz.

I left out the read part and the command based data select since it is about the principle and not the complete design.

[BrianHG]

Ok, first #1:

[BrianHG]

Ok, #2, lets start from scratch:

Code: [Select]

//---------------------------------------------------------------------------
//MCU interface for passing data in and out the FPGA based on a command set
//To avoid the loss of tri state control on the mcu databus, hierarchy is discarded

(*keep_hierarchy = "no"*)

module mcu_interface
(
  //Main clock input
  input i_main_clk,

  //Control input
  input i_clk,         //Clock signal:             Active low going pulse from the mcu to clock data
  input i_rw,          //Direction select signal:  Read 0 / Write 1
  input i_dc,          //Type select signal:       Data 0 / Command 1
 
  //Output to other module 
  output [31:0] o_channel1_negative_signal_step,
  output [31:0] o_channel1_positive_signal_step,
  output [31:0] o_channel2_negative_signal_step,
  output [31:0] o_channel2_positive_signal_step,

  //Data bus
  inout [7:0] io_data
);

  //---------------------------------------------------------------------------
  //Main registers

  reg [7:0] command;        //Stores the latest command
  reg [7:0] data_out;       //Stores data byte to be read by mcu
  reg [1:0] data_index;     //Index counter for multiple data byte handling

  //---------------------------------------------------------------------------
  //Command related data.

  reg [7:0] channel1_negative_signal_step[3:0];  //Command 0x00
  reg [7:0] channel1_positive_signal_step[3:0];  //Command 0x01
  reg [7:0] channel2_negative_signal_step[3:0];  //Command 0x02
  reg [7:0] channel2_positive_signal_step[3:0];  //Command 0x03


 
  //---------------------------------------------------------------------------
  //Sample the MCU control inputs to our system clock.

reg ireg_i_clk  ;
reg ireg_i_rw   ;
reg ireg_i_dc   ;
reg ireg_i_data ;
reg dly_i_clk  ;
reg dly_i_rw   ;
reg dly_i_dc   ;


always @(posedge i_main_clk) begin
                                ireg_i_clk  <= i_clk    ;
                                ireg_i_rw   <= i_rw     ;
                                ireg_i_dc   <= i_dc     ;
                                ireg_i_data <= io_data  ;

                                dly_i_clk  <= ireg_i_clk    ;
                                dly_i_rw   <= ireg_i_rw     ;
                                dly_i_dc   <= ireg_i_dc     ;
                                end


  //---------------------------------------------------------------------------
  //Ensure data_out is high Z during write.  i_rw == 1
  //And actual data for read.                i_rw == 0

  assign io_data = ireg_i_rw ? 8'bZ : data_out;

endmodule

//---------------------------------------------------------------------------
Why is this first step a good idea?
What did I just do?
Does it remind you of something else?
Why will the next step fix all your timing problems?

Now, everything you code next should minimize transitions on the MCI control port.
Everything looks to be 32bit controls.  You need to decide whether to latch 4bytes in a command,
or the entire 16 bytes in 1 shot.

[BrianHG]

Next step, add a 32bit clock in data register as well as triggers when and how that data should be stored.

Hint, this should only be 1 line.

[pcprogrammer]

With this first step you separate the external world from the internal FPGA making sure it is all in a known state for further processing. With the delayed signals you rule out the meta stability issue.

It is similar to having the DAC output clocked in the IO blocks to overcome the signal delays within the chip.

I'm leaning towards your idea with the transfer done signal from the MCU and have a 7 byte array for holding the command plus at max 6 bytes of data. On the other hand it might be better to have the step values for a single channel change at the same time, because otherwise the frequency can vary when the pulse width is changed.

But why is that article I pointed out stating it is better to have at least a second flip flop for making sure meta stability is kept out of the door, while you only use one?

[BrianHG]

But why is that article I pointed out stating it is better to have at least a second flip flop for making sure meta stability is kept out of the door, while you only use one?

They are assuming that that 125MHz clock is not the same speed as your system clock,
or,
That the enable out requires extra width since you may be sending data to a slower clock domain,
or,
That the data latch clock is in parallel with the data itself and you want the data to be all set and valid first.

We do not need the extra width and our source MCU toggles less than half the frequency of the 125MHz, so, once everything is already sampled on the 125Mhz bus, and we know the MCU sets the data clock latch on it's next cycle, we can assume the data on that next clock is truly valid and we can latch away treating it as synchronous data.

Now, for that 1 liner 32bit data register.

[pcprogrammer]

That is a tricky one.

From the MCU point of view four toggles are needed to get the data into the FPGA, which can be done with a shift register construction, but that is not a single line (well can be but it is multiple instructions)

And looking back on your example there is a need for an "if" to determine the when and possibly a case statement for deciding to which "variable" the data needs to be written.

I found out that it does not allow a byte array to be assigned with just the name.

Code: [Select]

reg [7:0] data_in[3:0];
reg [31:0] control_value;

assign control_value <= data_in;

Is not allowed, even though the number of bits is the same.

So I don't know how to do this in a single line.

[BrianHG]

Try this:

Code: [Select]

//---------------------------------------------------------------------------
  //Sample the MCU control inputs to our system clock.

reg ireg_i_clk           = 0 ;
reg ireg_i_rw            = 0 ;
reg ireg_i_dc            = 0 ;
reg ireg_i_data          = 0 ;
reg dly_i_clk            = 0 ;
reg dly_i_rw             = 0 ;
reg dly_i_dc             = 0 ;
reg [31:0] long_data_reg = 0 ;

// Choose one of the following
//wire shift_8_32_data = ( ireg_i_clk && !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load and shift in the new 8 bit data.
wire shift_8_32_data = ( ireg_i_clk !=  dly_i_clk ) ; // every time the dly_i_clk transitions high or low, load and shift in the new 8 bit data.

always @(posedge i_main_clk) begin
                                ireg_i_clk  <= i_clk    ;
                                ireg_i_rw   <= i_rw     ;
                                ireg_i_dc   <= i_dc     ;
                                ireg_i_data <= io_data  ;

                                dly_i_clk  <= ireg_i_clk    ;
                                dly_i_rw   <= ireg_i_rw     ;
                                dly_i_dc   <= ireg_i_dc     ;

                                if ( shift_8_32_data )  long_data_reg[31:0] <= { long_data_reg[23:0] , ireg_i_data[7:0]  } ;


                                end

Does this make sense to you?

[BrianHG]

Next, generate a load command trigger wire and point to a 16word 32bit array to store the command's sent address.

[pcprogrammer]

Well the { } construct is new to me, but I get the intention of it.

The full 32 bits register gets assigned the bits of the full register positions 23:0 in positions 31:8 and the 8 bits of the data register into the positions 7:0

Looks like a handy construct.

Guess I have to read the verilog manual again

Edit: I looked back into this tutorial http://www.asic-world.com/verilog/operators2.html and see it is the replication operator. That is the problem with aging, things don't stick in memory that easy anymore. Needs to be used multiple times before it registers.

[pcprogrammer]

Next, generate a load command trigger wire and point to a 16word 32bit array to store the command's sent address.

This is for tomorrow.

[BrianHG]

And don't forget, the load command, or you should have called the MCU input write_data should be written in 1 line, for all your 32bit registers.

[BrianHG]

Edit: I looked back into this tutorial http://www.asic-world.com/verilog/operators2.html and see it is the replication operator. That is the problem with aging, things don't stick in memory that easy anymore. Needs to be used multiple times before it registers.

Concatenation Operator...

Replicator Operator uses double { # {}} braces with a number in front...

[pcprogrammer]

Edit: I looked back into this tutorial http://www.asic-world.com/verilog/operators2.html and see it is the replication operator. That is the problem with aging, things don't stick in memory that easy anymore. Needs to be used multiple times before it registers.

Concatenation Operator...

Replicator Operator uses double { # {}} braces with a number in front...

Oh hell, I proved my own point. I scanned the article and found both the use cases of the braces, understood them and still provided the name of the wrong one

[pcprogrammer]

Code: [Select]

// Choose one of the following
//wire shift_8_32_data = ( ireg_i_clk && !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load and shift in the new 8 bit data.
wire shift_8_32_data = ( ireg_i_clk !=  dly_i_clk ) ; // every time the dly_i_clk transitions high or low, load and shift in the new 8 bit data.


I guess the following is also correct, but that you rather use the logical operator && instead of the bitwise operator & to avoid mistakes when the variable is multi bit.

Code: [Select]

wire shift_8_32_data = ( ireg_i_clk & !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load and shift in the new 8 bit data.


[BrianHG]

Code: [Select]

// Choose one of the following
//wire shift_8_32_data = ( ireg_i_clk && !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load and shift in the new 8 bit data.
wire shift_8_32_data = ( ireg_i_clk !=  dly_i_clk ) ; // every time the dly_i_clk transitions high or low, load and shift in the new 8 bit data.


I guess the following is also correct, but that you rather use the logical operator && instead of the bitwise operator & to avoid mistakes when the variable is multi bit.

Code: [Select]

wire shift_8_32_data = ( ireg_i_clk & !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load and shift in the new 8 bit data.


Whenever your result will be crunched down to a single bit, always use: and = &&, or = ||.

Since wire 'shift_8_32_data' is a single bit, it is safer to use && here.

If ireg_i_clk & dly_i_clk were 4 bits, the single & would make a 4 bit result, each individual bits anded together, but with a 1 bit destination wire, then only bits 0 and 0 of the sources would feed the output.

[pcprogrammer]

Code: [Select]

//---------------------------------------------------------------------------
//Sample the MCU control inputs to our system clock.

reg ireg_i_clk = 0 ;
reg ireg_i_rw = 0 ;
reg ireg_i_dc = 0 ;
reg dly_i_clk = 0 ;
reg dly_i_rw = 0 ;
reg dly_i_dc = 0 ;
reg [7:0] ireg_i_data = 0 ;
reg [31:0] long_data_reg = 0 ;
reg [7:0] command = 0;

wire clock_data = ( ireg_i_clk && !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load the data from the cpu

wire load_command = (dly_i_dc && dly_i_rw && clock_data);  //When dly_i_dc and dly_i_rw are high and there is an active clock transition load the command register

wire shift_8_32_data = (!dly_i_dc && dly_i_rw && clock_data ) ; //When dly_i_dc is low and dly_i_rw is high and there is an active clock transition load and shift in the new 8 bit data.

always @(posedge i_main_clk) begin
     ireg_i_clk  <= i_clk;
     ireg_i_rw   <= i_rw;
     ireg_i_dc   <= i_dc;
     ireg_i_data <= io_data;

     dly_i_clk  <= ireg_i_clk;
     dly_i_rw   <= ireg_i_rw;
     dly_i_dc   <= ireg_i_dc;

     if(load_command) command <= ireg_i_data;

     if ( shift_8_32_data )  long_data_reg[31:0] <= { long_data_reg[23:0] , ireg_i_data[7:0]  } ;

   end

Guess there is no need for "if else" constructs here since it is all parallel logic and not CPU wasting cycles on an "if" that won't be true if the other one is true.

With the other control lines mixed in to the "enable" wires it is more like the original setup. It needs more work of course to fully convert the original code to this proper code.

For sure a very useful exercise in making proper verilog, so thank you again BrianHG.

Regards,
Peter

[BrianHG]

Ok, problems:

Let's stick with 1 clock data.  Choose if you want to clock on the low or high, or both.

wire clock_data = ( ireg_i_clk && !dly_i_clk ) ; // every time the dly_i_clk transitions to high, load the data from the cpu

This means change this line:

    if ( clock_data )  long_data_reg[31:0] <= { long_data_reg[23:0] , ireg_i_data[7:0]  } ;

Now, next:

wire load_command = ( !dly_i_dc && ireg_i_dc ) && ( ireg_i_data < 8 ) ;  // When when dly_i_dc goes from low to high, load command so long as the command address is within range.

Now for the command reg:

reg [ 31:0 ] command [ 0:7 ] = '{default:0} ; // our command register memory, default to '0'.

now for the load command:

if (load_command) command[ ireg_i_data[ 2:0 ]] <= long_data_reg;


Correct your code and let's see if you can figure out how the MCU writes to each of your 8 control regs.

[pcprogrammer]

I see that you are thinking in an other direction and are looking at the dly_i_dc signal as another trigger signal coming from the MCU.

In your setup all the data from the MCU is clocked into the 32 bit register long_data_reg and then used based on other events.

With the load_command wire being based on ireg_i_data < 8 the MCU has to leave the dly_i_clk as is and then load 0 - 7 on the data bus to address the intended target.
Then it needs to pulse the dly_i_dc high.

This will load the command register indexed by the lower three bits of the ireg_i_data register with the previously loaded data.

So the actions the MCU needs to take:
Put the most significant byte of the data on the bus (Lets say step_control_1 >> 24)
Pulse the data clock line high (Can be high - low - high or low - high - low as long as there is a rising edge)
Put the next byte on the data bus (step_control_1 >> 16)
Pulse the data clock line high
Put the next byte on the data bus (step_control_1 >> 8 )
Pulse the data clock line high
Put the least significant byte on the data bus (step_control_1)
Pulse the data clock line high

Put 0 on the data bus (Assume command 0 is step_control_1)
Pulse the data command line (dly_i_dc) high (Can also be high - low - high or low - high - low as long as there is a rising edge)

Repeat the above for the other commands with the correct command number.

Edit: corrected the smiley to be 8 )

[pcprogrammer]

Since you dismiss the setup I wrote with clocking a command or data register based on a MCU control signal, is there a timing risk with the combining the MCU control signals into multiple clock enables?

[BrianHG]

Ok, post #1 above, correct.

Since you dismiss the setup I wrote with clocking a command or data register based on a MCU control signal, is there a timing risk with the combining the MCU control signals into multiple clock enables?

What do you mean by multiple clock enables?
The moment the 'ireg_i_dc' goes high, on the next clock, that 'command[address]', all 32 bits will get a copy of the last clocked in data in a single 125MHz step.

If you need larger reg controls, you may expand the 'command' and 'long_data_reg' to 64 bits.  Remember the direction we are filling the clocked 8 bit data.  If command 5 only needs to be 8 bit, just clock in 1 data byte and then do a load command right after.  If command 6 needs to be 64 bit, then just clock in 8 bytes and then do the load command 6.  Internally in the FPGA, you only wire the # of bits of each command # you wish to use.  The compiler will automatically prune out all unused bits in all the command numbers, so nothing gets wasted in FPGA logic cells.

Let's see what your code looks like now?

If everything is to your liking, we will next do the 'read data', but after I get some sleep.

[pcprogrammer]

If everything is to your liking, we will next do the 'read data', but after I get some sleep.

  I was wondering when you did that, sleeping. You seemed to be active all day yesterday.

What do you mean by multiple clock enables?

What I meant is that I used the dly_i_clk transition combined with the dly_i_dc signal being low as an enable signal for loading a data register and the dly_i_clk transition combined with the dly_i_dc signal being high as an enable signal for loading a command register.

Code: [Select]

wire clock_data = ( ireg_i_clk && !dly_i_clk ) ;                               //Every time the dly_i_clk transitions to high, load the data from the cpu
wire load_command = (dly_i_dc && dly_i_rw && clock_data);      //When dly_i_dc and dly_i_rw are high and there is an active clock transition load the command register
wire shift_8_32_data = (!dly_i_dc && dly_i_rw && clock_data ) ;  //When dly_i_dc is low and dly_i_rw is high and there is an active clock transition load and shift in the new 8 bit data.

The down side with this is the need of a state machine like setup to determine when to load the data into the intended target register, or a case on command statement in the shift_8_32_data part to shift the data into the intended target.

There are many roads in to Rome, so most likely also many ways of doing a thing like this even within the proper realm of a single master clock.

[pcprogrammer]

I guess you already answered my question about the multiple clock enables.

The construct for handling the command uses a similar construct were a "static" signal is used to make an "enable" signal

Code: [Select]

wire load_command = ( !dly_i_dc && ireg_i_dc ) && ( ireg_i_data < 8 ) ;

Not sure what is wrong with the code so far. It compiles and some simulation showed promising signals, but in the actual hardware something is wrong. Measured the data strobe made by the MCU and it is slow enough. The pulse is low for 100ns so should be ok.

I did increase the registers to be 48 bits instead of 32 bits.

Have to implement some debug signals to the led's on the FPGA board to see what is going on.

Edit:
Using the leds showed that the MCU interface is working as intended.

Added a two bit counter in the "if(shift_8_48_data)" part and it counts every data strobe. Moved the two bit counter into the "if(load_into_control_reg)" part and that gives the same result. A count whenever the control strobe is pulsed. (Also renamed the signals)

I then assigned the debug leds to compares with the data registers. This shows that the data is also written as intended. Led off when below value and on when above value.

But the expected saw tooth on the output does not come. So have to scale back to 32 bits to see if that is the problem.

Edit 2:
Changed back to 32 bits and it worked. But while making that change I noticed I did not used the correct bits for the DAC output in the 48 bits variant. Modified it back to 48 bits again with the correct bits for the DAC output and it works like a charm.

[BrianHG]

If everything is to your liking, we will next do the 'read data', but after I get some sleep.

  I was wondering when you did that, sleeping. You seemed to be active all day yesterday.

What do you mean by multiple clock enables?

What I meant is that I used the dly_i_clk transition combined with the dly_i_dc signal being low as an enable signal for loading a data register and the dly_i_clk transition combined with the dly_i_dc signal being high as an enable signal for loading a command register.

Code: [Select]

wire clock_data = ( ireg_i_clk && !dly_i_clk ) ;                               //Every time the dly_i_clk transitions to high, load the data from the cpu
wire load_command = (dly_i_dc && dly_i_rw && clock_data);      //When dly_i_dc and dly_i_rw are high and there is an active clock transition load the command register
wire shift_8_32_data = (!dly_i_dc && dly_i_rw && clock_data ) ;  //When dly_i_dc is low and dly_i_rw is high and there is an active clock transition load and shift in the new 8 bit data.


wire load_command = ( dly_i_dc && dly_i_rw && clock_data );      //When dly_i_dc and dly_i_rw are high and there is an active clock transition load the command register

Somewhat ok.

For this to work, (dly_i_dc && dly_i_rw) must be guaranteed high by the time clock_data transitions high.  The means that on your MCU, on one clock, you need to set (dly_i_dc && dly_i_rw), then on the next clock, set 'clock_data' to ensure that the first have truly been gauranteed registered before you run the data clock to run the 'load command'.

This is because with the 125 MHz clock, you must ensure that the MCU outputs are within the same picosecond going into the FPGA, otherwise, with the 2 async clocks and difference in MCUs IOs and board tracing, you will hit a lemon sample time again and again.

Now, if you are writing to a single port on the MCU to drive all these 3 signals simultaneously in a single MCU port write data command, there is a fix.  After the wire clock_data =...  Make a new 'reg clock_data_dly = 0' and inside the always, make clock_data_dly <= clock_data.  Now in the above wire 'load_command', use the clock_data_dly.  Since we have an 8ns 125MHz clock, this means that (dly_i_dc && dly_i_rw) can be as late as 8ns at the FPGA inputs before the clock_data input allowing the 'load_command' to still function.

[BrianHG]

Not sure what is wrong with the code so far. It compiles and some simulation showed promising signals, but in the actual hardware something is wrong. Measured the data strobe made by the MCU and it is slow enough. The pulse is low for 100ns so should be ok.

That's huge and should be seen by a 8ns clock.

Looks like it's time to see what you have done, correct if needed, then make a read back data.

I do not know about your FPGA tools, but with Altera, Lattice & Xilinx, they all have a real-time 'signal-tap' tool which runs on their J-Tag programmer.  It provides a real-time, onscreen logic analyzer of any internal regs inside your FPGA while running.  Without silly LED's, doesn't your FPGA compiler have such a tool.  With it, you can see the MCU inputs in real time and trigger a screen capture of your regs as the data comes in.  The logic analyzer should run at your system 125MHz speed, so you wont miss a beat.\ and you should be able to capture a full reg being sent by your MCU.

[pcprogrammer]

The IDE does have some chip probe and chip watcher, so it might well be possible to do what you described.

The documentation for the IDE is not that great. Had to translate it from Chinese to English with Google translate and then still it is not very good, but I will have a look into this chip probe option.

Edit:
The chip probe option in the IDE allows the user to connect internal signals to IO pins without having to add code to the HDL, but it is needed to create a new bit stream, so similar to debugging with LED's.

[BrianHG]

Line 40 in MCU_Interface.v

  reg [47:0] control_data_reg[0:4];

Little typo on your part, that 4 should be a 3.


Ok, now for the read.  Step 1, make a wire which grabs the read address.  Could something like this work for you:

  wire read_from_control_reg = ( ireg_i_control_strobe && !dly_i_control_strobe && ireg_i_data[7] ) && ( ireg_i_data[1:0] < 4 ) ;

What I have done here is if you send a control reg with the address msb bit 7 is high, we will trigger a snap of the read address 0-3.  With this upper address bit set, no writes will by the 'load_into_control_reg'.

If this is ok, you will then need to generate a 48bit 'long_data_reg_read' reg and lets see if you can make that 1 line IF which captures the data.

Then we will have to deal with passing the output enable during the read.
And copying the 'long_data_reg_read' into an 8 bit output 'oreg_data_out'.  This DFF copy is to help internal FPGA routing and FMAX timing before reaching the IO port.
Shifting the 'long_data_reg_read' once data clock.

And see if you can work out the best means of avoiding data bus IO output enable conflict.

Again, there should be a total of something like 4 new lines of code total, and your MCU interface should be done.

[pcprogrammer]

Yeah typo's easily made

Sure "address" bit 7 is good for me to setup for reading. Will write something up and see if I can figure it out. For this I also have to change the software on the F1C100s. Should not be a problem. Already made the changes for the new write method.

The 100ns pulse is seemingly long but could be a limit of the internal F1C100s buses. The code to make the pulses and read or write is:

Code: [Select]

//FPGA port registers (Port E on the F1C100s)
#define FPGA_BUS_CFG_REG            ((volatile uint32 *)(0x01C20890))
#define FPGA_CTRL_CFG_REG           ((volatile uint32 *)(0x01C20894))
#define FPGA_DATA_REG               ((volatile uint32 *)(0x01C208A0))

//Initialize the control lines for communication with the FPGA (PE10:12 output)
#define FPGA_CTRL_INIT()            (*FPGA_CTRL_CFG_REG = (*FPGA_CTRL_CFG_REG & 0xFFF000FF) | 0x00011100)

//Initialize the two strobe lines to high
#define FPGA_CLK_INIT()             (*FPGA_DATA_REG |= 0x00001400)

//Strobe signals for data or control actions
#define FPGA_PULSE_DATA_STROBE()    (*FPGA_DATA_REG &= 0xFFFFFBFF);(*FPGA_DATA_REG |= 0x00000400)
#define FPGA_PULSE_CONTROL_STROBE() (*FPGA_DATA_REG &= 0xFFFFEFFF);(*FPGA_DATA_REG |= 0x00001000)

//Control the direction of the FPGA data bus and tell the FPGA to control it's High Z state
#define FPGA_BUS_DIR_IN()           (*FPGA_DATA_REG = (*FPGA_DATA_REG & 0xFFFFF7FF) | 0x00000000);(*FPGA_CTRL_CFG_REG &= 0xFFFFFF00);(*FPGA_BUS_CFG_REG &= 0x0F000F00);
#define FPGA_BUS_DIR_OUT()          (*FPGA_DATA_REG = (*FPGA_DATA_REG & 0xFFFFF7FF) | 0x00000800);(*FPGA_CTRL_CFG_REG &= 0xFFFFFF00);(*FPGA_BUS_CFG_REG &= 0x0F000F00);(*FPGA_CTRL_CFG_REG |= 0x00000011);(*FPGA_BUS_CFG_REG |= 0x10111011)

//Put data on or get data from the FPGA data bus
#define FPGA_SET_DATA(x)            (*FPGA_DATA_REG = (*FPGA_DATA_REG & 0xFFFFFC44) | ((x & 0x000000E0) << 2) | ((x & 0x0000001C) << 1) | (x & 0x00000003))
#define FPGA_GET_DATA()             (((*FPGA_DATA_REG & 0x00000380) >> 2) | ((*FPGA_DATA_REG & 0x00000038) >> 1) | (*FPGA_DATA_REG & 0x00000003))

The FPGA_PULSE_DATA_STROBE is just an AND followed by an OR.

The chip viewer in the IDE seems to be what you described. It looks like it is also needed to rebuild the bit stream and load it to make it work. Attached is a screen cap of the work flow described in the manual.

[pcprogrammer]

Guess you also made a typo.

wire read_from_control_reg = ( ireg_i_control_strobe && !dly_i_control_strobe && ireg_i_data[7] ) && ( ireg_i_data[1:0] < 4 ) ;

The last bit in red will always be true since max value of two bits is three. I will use [6:0] instead.

[BrianHG]

The 100ns pulse is seemingly long but could be a limit of the internal F1C100s buses. The code to make the pulses and read or write is:

When dealing with bit-blasting, MCUs and C code, you unfortunately need to view the assembly listing for the code and the look at the MCU's available instruction set.

In my case when using DS-PIC MCU, for bit blasting, I would usually create my own little #asm routine in C to send and receive bytes from a port.

Properly using indirect addressing reg with IO port's bit sec & clear, I can stream out huge words using 2-3 instruction cycles per byte VS the C version which would have done something like 10-15 cpu cycles per byte.

[BrianHG]

Guess you also made a typo.

wire read_from_control_reg = ( ireg_i_control_strobe && !dly_i_control_strobe && ireg_i_data[7] ) && ( ireg_i_data[1:0] < 4 ) ;

The last bit in red will always be true since max value of two bits is three. I will use [6:0] instead.

Correct...

[pcprogrammer]

The 100ns pulse is seemingly long but could be a limit of the internal F1C100s buses. The code to make the pulses and read or write is:

When dealing with bit-blasting, MCUs and C code, you unfortunately need to view the assembly listing for the code and the look at the MCU's available instruction set.

In my case when using DS-PIC MCU, for bit blasting, I would usually create my own little #asm routine in C to send and receive bytes from a port.

Properly using indirect addressing reg with IO port's bit sec & clear, I can stream out huge words using 2-3 instruction cycles per byte VS the C version which would have done something like 10-15 cpu cycles per byte.

Sure when speed is needed assembler might be better. The code I use is compiled with -O3 so should be fairly optimized. For this project there is no need for high speed.

In my early embedded computing day's I only wrote in assembler. Z80, 6502, 8051, etc. All 8 bit CPU's and fairly simple assembler. ARM is not super difficult but still not something I shake out of my sleeve. (Translated Dutch expression meaning it takes a bit of effort)

[pcprogrammer]

Not sure if this how you have it in your mind.

Have to dive into test benches to do more with simulation. At least no risk in damaging hardware.

And thus I have not tested it in the hardware. Also getting tired, so tomorrow is a new day

But so far, again a big thanks to you BrianHG.

Cheers,
Peter

[BrianHG]

Not sure if this how you have it in your mind.

Have to dive into test benches to do more with simulation. At least no risk in damaging hardware.

And thus I have not tested it in the hardware. Also getting tired, so tomorrow is a new day

But so far, again a big thanks to you BrianHG.

Cheers,
Peter

Ok, not bad...  Let's examine:


      if(shift_48_8_data)
        begin
          oreg_o_data <= long_data_reg_read[7:0];        
         
          long_data_reg_read <= { 8'h0, long_data_reg[47:8] };
        end

      //When enabled load the received data into the intended control register
      if(load_into_control_reg) control_data_reg[ireg_i_data[1:0]] <= long_data_reg;     
     
      //When enabled read the addressed control register into the temporary register     
      if(read_from_control_reg) long_data_reg_read <= control_data_reg[ireg_i_data[1:0]];


Looking what I have listed in red, you can see that there are 2 potential simultaneous times where we assign a value to 'long_data_reg_read '.  Now I know this is permitted, but my personal preference would make the line a little more exclusive and to be written like this:

Code: [Select]

      //When enabled read the addressed control register into the temporary register     
      if      (read_from_control_reg) long_data_reg_read <= control_data_reg[ireg_i_data[1:0]];
      else if (shift_48_8_data)       long_data_reg_read <= { 8'h0, long_data_reg[47:8] };

I find this so much easier to consume mentally and we see a true order of preference.

Ok, looking at the orange, for that line, just move it to line 38 and type it like this:

Code: [Select]

always@(posedge i_main_clk) oreg_o_data <= long_data_reg_read[7:0];
This is nothing more than a simple DFF chain to the output FF, no special conditions attached.

For line 39, add:

Code: [Select]

assign io_data = ireg_i_read_write_select ? oreg_o_data  : 8'bz ;
Yes, that's it.  You MCU read & write interface is finished.

ireg_i_read_write_select  is asynchronous, it basically is a direction control for the data bus.  It does nothing else.

Now, show me your MCU read bit-blast order...

[BrianHG]

Also, invert your 'ireg_i_read_write_select  ' so that low equals write data to the fpga, high equals read data from fpga.  This way at power-up, the FPGA will have the 8bit data bus OE disabled.

[pcprogrammer]

Looking what I have listed in red, you can see that there are 2 potential simultaneous times where we assign a value to 'long_data_reg_read '.  Now I know this is permitted, but my personal preference would make the line a little more exclusive and to be written like this:

Did not think about this one. I agree it is better to avoid such a possibility.

Ok, looking at the orange, for that line, just move it to line 38 and type it like this:

Code: [Select]

always@(posedge i_main_clk) oreg_o_data <= long_data_reg_read[7:0];
This is nothing more than a simple DFF chain to the output FF, no special conditions attached.

I have thought about this one. Was not sure if constantly clocking it into a DFF would be ok, but I guess that as long as the input is stable it will not do anything.
Is it placing it in it's own always@ any different from having it in the big always@ with the "if" parts?

Now, show me your MCU read bit-blast order...

Steps for reading:
Load the data bus with 0x80 | control_register_address
Pulse the control strobe
Setup the data bus as input
Set the read_write_select to reading
Read the least significant byte

When more bytes need to be read:
Pulse the data strobe
Read the next byte

Repeat the last two steps until done.

Edit: Had the order of bus mode change wrong. MCU bus has to be set as input first before having the FPGA output the data.

[pcprogrammer]

Also, invert your 'ireg_i_read_write_select  ' so that low equals write data to the fpga, high equals read data from fpga.  This way at power-up, the FPGA will have the 8bit data bus OE disabled.

Since the external MCU FPGA connection lacks pull-up's or pull-down's, this should be the case as long as the input state of the MCU with which it starts is seen as a zero.

Probably also the best state to have them both in, power wise. Shall not be a lot of power, but on a battery every micro amp counts.

[BrianHG]

Looking what I have listed in red, you can see that there are 2 potential simultaneous times where we assign a value to 'long_data_reg_read '.  Now I know this is permitted, but my personal preference would make the line a little more exclusive and to be written like this:

Did not think about this one. I agree it is better to avoid such a possibility.

Ok, looking at the orange, for that line, just move it to line 38 and type it like this:

Code: [Select]

always@(posedge i_main_clk) oreg_o_data <= long_data_reg_read[7:0];
This is nothing more than a simple DFF chain to the output FF, no special conditions attached.

I have thought about this one. Was not sure if constantly clocking it into a DFF would be ok, but I guess that as long as the input is stable it will not do anything.
Is it placing it in it's own always@ any different from having it in the big always@ with the "if" parts?

You can remove the 'always' from that line and place just the 'oreg_o_data <= long_data_reg_read[7:0]; ' anywhere inside your main existing always@(posedge...).

You should not place it inside an 'if' block anywhere.  If you did, for example place it inside the data strobe clock's if(), the oreg_o_data will only be triggered once every data clock strobe.  This means an additional data clock strobe before you read the first byte.

Now, show me your MCU read bit-blast order...

Steps for reading:
Load the data bus with 0x80 | control_register_address
Pulse the control strobe
Setup the data bus as input
Set the read_write_select to reading

***** just realize this may take up to 16ns after the MCU output control toggles + the input delay or your MCU before you should read your IO port's data input.

Read the least significant byte

When more bytes need to be read:
Pulse the data strobe
Read the next byte

Repeat the last two steps until done.

Edit: Had the order of bus mode change wrong. MCU bus has to be set as input first before having the FPGA output the data.

[pcprogrammer]

***** just realize this may take up to 16ns after the MCU output control toggles + the input delay or your MCU before you should read your IO port's data input.

Easy solution I see for this is have the FPGA strobe on the falling instead of the rising edge of the MCU signal. With the pulse being 100ns wide these 16ns are long past before the MCU is ready to read the data.

A later experiment in optimizing the MCU code for a more high speed transfer will be fun. That is what hobby is about, fun

I have learned a lot from this session.

Thanks and cheers,
Peter

[BrianHG]

These signals are combinational logic, un-buffered or un-registered.  Your FMAX may suffer.

Code: [Select]

  //On control strobe and control address 0x2F reset the phase register 
  wire reset_signal_phase_registers = ( !ireg_i_control_strobe && dly_i_control_strobe ) && ( ireg_i_data == 8'h2F );
 
  //On control strobe and control address 0x2E load the channel 2 step registers
  wire load_channel2_step_registers = ( !ireg_i_control_strobe && dly_i_control_strobe ) && ( ireg_i_data == 8'h2E );
 
  //On control strobe and control address 0x2D add the channel 2 signal phase
  wire add_channel2_signal_phase    = ( !ireg_i_control_strobe && dly_i_control_strobe ) && ( ireg_i_data == 8'h2D );
 
  //On control strobe and control address 0x2C load the channel 1 step registers
  wire load_channel1_step_registers = ( !ireg_i_control_strobe && dly_i_control_strobe ) && ( ireg_i_data == 8'h2C );
 
  //On control strobe and control address 0x2B add the channel 1 signal phase
  wire add_channel1_signal_phase    = ( !ireg_i_control_strobe && dly_i_control_strobe ) && ( ireg_i_data == 8'h2B );

And their associated assigns at the bottom of your code:

Code: [Select]

  assign o_reset_signal_phase_registers = reset_signal_phase_registers; 

  assign o_load_channel1_step_registers = load_channel1_step_registers;
  assign o_add_channel1_signal_phase    = add_channel1_signal_phase;

  assign o_load_channel2_step_registers = load_channel2_step_registers;
  assign o_add_channel2_signal_phase    = add_channel2_signal_phase;

A safer bet would be:

Code: [Select]

...
 reg [255:0] command_pulse = 256'd0 ;
...
 wire load_any_command  = ( !ireg_i_control_strobe && dly_i_control_strobe ) ;
...
      if(load_any_command) command_pulse [ireg_i_data] <= 1'b1;    // Set a single bit to 1.
      else                 command_pulse               <= 256'd0;  // otherwise, clear all bits.
...

  assign o_reset_signal_phase_registers = command_pulse [ 8'h2f ] ;
  assign o_load_channel1_step_registers = command_pulse [ 8'h2e ] ;
  assign o_add_channel1_signal_phase    = command_pulse [ 8'h2d ] ;
  assign o_load_channel2_step_registers = command_pulse [ 8'h2c ] ;
  assign o_add_channel2_signal_phase    = command_pulse [ 8'h2b ] ;

Now, these o_reset_xxxx / o_load/add_xxx are registered, inside your MCU interface, coming out as a clean single flip-flop's 'Q' for each wire instead of a combinational stack of gates.