[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xylo-SDR] Your FPGA Code
Hi Phil,
Here are some Verilog "tips." I'm cc'ing the reflector, too, in case anyone
else is interested.
Regarding your code, here are some coding recommendations that I've found
helpful in miminimizing unintentional errors:
1. If the FPGA has a reset signal going to it, be sure to use it to
initialize your registers. Otherwise, assume that the registers will
power-up in random states and plan accordingly. Setting "count = 0" in your
code won't guarantee that count will be 0 at power-up.
For example, if you have a low-true Reset input (nrst), your state machine
code could look something like this...
reg [2:0] state0;
reg [7:0] q;
reg [7:0] fifo_data;
reg [5:0] count
always @ (posedge FX2_CLK or negedge nrst) begin
if (~nrst) begin // reset our variables
state <= 3'h0;
q <= 8'h0;
fifo_data <= 8'h0;
count <= 5'h0;
end
else begin // out-of-reset operation...
case(state)
0: if (LRCLK) state <= 1; // wait for LRCLK to go high
........(etc).
endcase;
end
end
In the above example, 'nrst' acts as an asynchronous clear.
2. In your state machine's case statement, your variables aren't all
defined in each of the case statement's selections. For example, let's look
at the following code:
case(state)
0: if (LRCLK) state <= 1; // wait for LRCLK to go high
1: if (BCLK) state <= 2; // wait for Wolfson clock to go high
2: begin
q[7:1] <= q[6:0]; // shift current data left
q[0] <= DOUT; // get the new data
count <= count + 1; // increment the counter
state <= 3; // next state
end
(...etc.)
In states 0 & 1 you only define the state variable. And in state 2 you also
define 'q' & 'count'. There's no inherent problem with this, but you're
depending upon the compiler to make some assumptions regarding how you'd
like to have variables treated when you haven't explicitely defined them,
and, unless you're careful, the compiler may do something else than what
you're assuming it's going to do, and this can be a pain to troubleshoot.
Therefore, I always make sure that all variables ae defined for each case
selection. For example, the code could look something like this...
case(state)
0: begin // wait for LRCLK to go high
if (LRCLK) begin
state <= 1;
q <= q;
count <= count;
fifo_data <= fifo_data;
end
else begin
state <= 0;
q <= q;
count <= count;
fifo_data <= fifo_data;
end
end
1: begin // wait for Wolfson clock to go high
if (BCLK) begin
state <= 2;
q <= q;
count <= count;
fifo_data <= fifo_data;
end
else begin
state <= 1;
q <= q;
count <= count;
fifo_data <= fifo_data;
end
end
2: begin
q[7:1] <= q[6:0]; // shift current data left
q[0] <= DOUT; // get the new data
count <= count + 6'd1; // increment the counter
state <= 3; // next state
fifo_data <= fifo_data;
end
(...etc.)
In other words, it's a good habit to always define all variables for each
possible state. It doesn't so much matter for your example, but if/when you
start designing more complex state machines and other logic, it can save you
from grief. Yes, it does make the code longer, but it won't impact how it's
implemented within the FPGA.
3. I see that you're creating a write-strobe using combinatorial logic:
assign FX2_SLWR = ~(state == 4 && FX2_flags[2]);
Be aware that combinatorial logic can glitch, and unless your write-strobe
doesn't care about possible glitches, I'd strongly recommend you make this a
synchronous signal - in fact, it's something that could be easily imbedded
within your state machine.
(By the way - V1.4.4 of the SDR1K code suffers from this problem and it
caused erratic behaviour in my machine when I changed the preamp settings.
The code was fixed in later Preview versions).
4. Other tips:
4.1 Your shift register is good. For future reference, you can also
implement it using Verilog's concatenation expression:
q <= {q[7:1],DOUT};
The one thing you DON'T want to do is this:
q <= q << 1;
q[0] <= DOUT;
(Been there, done that!)
4.2 Quartus will give you a warning for the following expression:
count <= count + 1;
To remove the warning, make sure that the '1' is the same size as count.
For example, given that 'count' is a 6-bit number, you could write the
expression as follows:
count <= count + 6'd0; // (or 6'b000000, or 6'h00. d is decimal
representation, b is bit, h is hex)
4.3 If not already apparent, you can design combinatorial logic using
either the 'always' statemet or the 'assign' statement. If using 'assign',
you'll need to define your variables as 'wire's. If using 'always', you'll
need to define your variables as 'reg's. Here are two examples of the same
thing:
wire nWrite_strobe;
assign nWrite_strobe = ~((state == 3'd4) && FX2_flags[2]);
reg nWrite_strobe;
always@ (state or FX2_flags) begin
if ((state == 3'd4) && FX2_flags[2]) nWrite_strobe <= 1'b0;
else nWrite_strobe <= 1'b1;
end
4.4 I'm a big believer in commenting code, and you've done a good job. If
you've ever had to debug someone else's design, or looked at code you might
have written years before while scratching your head and muttering, "What
the hell was I thinking?", you'll know what I mean. This doesn't mean that
every line should be commented (unless you've done something very obscure or
very clever), but, in my opinion, the basic idea should be clearly
expressed.
Enjoy your verilog coding! It's alot of fun, as I'm sure you're
discovering!
- Jeff, WA6AHL
-----Original Message-----
From: Phil Harman [mailto:pvharman@arach.net.au]
Sent: Saturday, December 10, 2005 7:56 PM
To: Jeff Anderson
Subject: Re: Your FPGA Code
Hi Jeff,
Thanks for your help, much appreciated. I have been using the "Blue Arrow
w/Sq. Wave" button and ran simulations before but I did not check any of the
settings you suggested - I'll do that now.
I have found my problem. I was using the FX2 12/24/48MHz clock rather than
the 24.576MHz clock to drive my state machine. Hence I was getting
undefined states.
I don't really want to use this clock so will revert to my previous code
that did not use a state machine.
Please send all the code suggestions that you have - always ready to lean!
Perhaps you could post these on the Xylo-SDR forum so others can lean to.
Really appreciate your help.
73's Phil... VK6APH
----- Original Message -----
From: "Jeff Anderson" <jca1955@sbcglobal.net>
To: "Phil Harman" <pvharman@arach.net.au>
Sent: Saturday, December 10, 2005 11:16 PM
Subject: Your FPGA Code
> Hi Phil
>
> I took a look at your FPGA code and noticed that, although you had created
> a
> .VWF file for simulating your logic, you hadn't yet run it.
>
> It's a great tool for debugging designs and I highly recommend it. If you
> haven't yet tried running a simulation, it's pretty straightforward.
> Given
> your existing file, all you need to do is:
>
> 1. Start "Analysis & Synthesis" by clicking on the "Start Analysis &
> Synthesis" button in the toolbar (it's the middle button of the three
> purple
> buttons).
>
> 2. Create a Functional Simulation Netlist by:
> 2.1 Go to the "Processing" Menu.
> 2.2 Select "Generate Functional Simulation Netlist."
>
> 3. Setup the Simulator Tool by:
> 3.1 Go to the "Tools" Menu.
> 3.2 Select "Simulator Tool".
> 3.3 When the Simulator Tool launches, set "Simulation Mode" (it's the very
> top line) to "Functional" (instead of Timing).
> 3.4 Select the .VWF file you'd like to use for simulations in the
> "Simulation Input" window.
> (3.5 I usually also check the box that says "Overwrite Simulation
> nput"... )
>
> 4. Then, either press the START button on the Simulator Tool, or press
> the
> "Blue Arrow w/Sq. Wave" button in the toolbar, and away you go!
>
> By the way - I'm curious. What's the frequency of FX2 clock, and what
> sample rate are you running the codec at?
>
> Also - would you be open to some coding suggestions? There are some
> things
> you're doing which will probably work fine, but if you're not careful,
> they
> can bite you (been there, been bitten...).
>
> 73,
>
> - Jeff, WA6AHL
>
>
>
>
> --
> No virus found in this incoming message.
> Checked by AVG Free Edition.
> Version: 7.1.371 / Virus Database: 267.13.13/197 - Release Date: 9/12/2005
>
>
--
No virus found in this outgoing message.
Checked by AVG Free Edition.
Version: 7.1.371 / Virus Database: 267.13.13/197 - Release Date: 9/12/2005