VHDL: couldn't implement registers for assignments on this clock edge

Question

I'm getting the following errors:
Error (10822): HDL error at pwm.vhd(15): couldn't implement registers for assignments on this clock edge
Error (10822): HDL error at pwm.vhd(18): couldn't implement registers for assignments on this clock edge

Obviously the problem is the two rising edges that both change 'output'. How could I fix this problem?
code:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use ieee.std_logic_unsigned.all;
entity PWM is
 port (
 button1, button2 : in STD_LOGIC;
 output : inout STD_LOGIC_VECTOR(7 downto 0) := "00000000"
 );
end PWM;
architecture behavioral of PWM is
begin
 process (button1, button2)
 begin
 if rising_edge(button1) then
 output <= output + 1;
 end if;
 if rising_edge(button2) then
 output <= output - 1;
 end if;
 end process;
end behavioral;

• Nick Williams: Any particular reason the output is bidirectional?
• gilianzz: I wouldn't be able to say output <= output + 1; because you can't read the value of 'output' if it is of mode 'out'.
• Nick Williams: Ok, I'll make an answer that attempts to answer your question

Answer 1

The circuit you describe is a register with two input clocks, which doesn't really exist. There are DDR registers, but that is not what you described.

Futhermore, clocks are very special in a FPGA, and must be used with special care. A button is not a clock. Although it is possible to use normal signal as a clock, it is not recommended.

What you need is a real clock to drive your circuit, every board has one! Then, you need to detect the rising edges of your button according to that clock domain:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
--use ieee.std_logic_unsigned.all; Do not use with numeric_std
entity PWM is
port(
 clk : in STD_LOGIC;
 button1, button2 : in STD_LOGIC;
 output : out STD_LOGIC_VECTOR(7 downto 0)
);
end entity PWM;
architecture behavioral of PWM is
 signal button1_r : std_logic_vector(2 downto 0);
 signal button2_r : std_logic_vector(2 downto 0);
 signal output_i : unsigned(7 downto 0) := (others => '0');
begin
 process(clk)
 begin
 if rising_edge(clk) then
 -- Shift the value of button in button_r
 -- The LSB is unused and is there solely for metastability
 button1_r <= button1_r(button1_r'left-1 downto 0) & button1;
 button2_r <= button2_r(button2_r'left-1 downto 0) & button2;
 if button1_r(button1_r'left downto button1_r'left-1) = "01" then -- Button1 rising
 output_i <= output_i + 1;
 elsif button2_r(button2_r'left downto button2_r'left-1) = "01" then -- Button2 rising
 output_i <= output_i - 1;
 end if;
 end if;
 end process;
 output <= std_logic_vector(output_i);
end architecture behavioral;

Several things to note:

• The output is controlled by a single clock
• The output is an out, not an inout. inout is a tristate buffer, which you don't need. We use an internal signal to represent the output, and assign the output to it
• Whenever an asynchronous signal (like your button) to a clock is used in that clock domain, metastability is a problem. I suggest you read on it, but it is solved by using two registers in row, the output of the first register (button1_r(0)) must not be used.

Finally, physical buttons needs debouncing. As you push on it, the electrical connection goes on and off multiple times while the physical switch reaches its final position. Thus, it's likely that multiple rising edges of the buttons will be detected when you push it, incrementing the counter more than once.

• Nick Williams: You win this time!
• gilianzz: Are there ways to solve the pushbutton problem? Also: are there ways to detect the rising edge without a clock?
• Jonathan Drolet: Yes, search for "fpga debouncing". There are circuits solution (low-pass filter), but I've never seen that on a "real" board since it usually need a large capacitor. The FPGA solution is to check the button for activity every 10 ms (or some other value) instead of every clock cycle. Since it's human input, it's safe to put a long delay. As long as the button's press/unpress events happens more than 10ms appart, the mechanical glitches will resolve between two 10ms checks.

Answer 2

I wanted to make this a comment but I can't write clean looking VHDL there.

I'm just shooting from the hip but the way you handled the output is a bit unorthodox and I suspect that might be your problem, because otherwise: your VHDL is pretty simple and I don't see anything wrong with the rest of it.

This is what I would write personally:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use ieee.std_logic_unsigned.all;
entity PWM is
 port(
 button1, button2 : in STD_LOGIC;
 output : out STD_LOGIC_VECTOR(7 downto 0) //Change this to just output
 );
end PWM;
architecture behavioral of PWM is 
 //Use a signal to act as a buffer that writes to your output
 signal s_output : STD_LOGIC_VECTOR(7 downto 0) := X"00";
begin
 process(button1, button2)
 begin
 if rising_edge(button1) then
 s_output <= s_output + 1; //Instead of directly manipulating output, change the signal
 end if;
 if rising_edge(button2) then
 s_output <= s_output - 1; //Instead of directly manipulating output, change the signal
 end if;
 end process;
 output <= s_output //Assign the signal to output after the process
end behavioral;

I put comments where I made changes to your code.

Basically, you use a signal that acts as a buffer to your output. You use the signal for manipulating and changing, then when you're done with the process: assign the signal to the output.