Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AxiSpiMaster - Fix bug in shadow RAM for multi-chip configurations #1205

Draft
wants to merge 2 commits into
base: pre-release
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions protocols/spi/rtl/AxiSpiMaster.vhd
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ entity AxiSpiMaster is
axiWriteMaster : in AxiLiteWriteMasterType;
axiWriteSlave : out AxiLiteWriteSlaveType;
-- Copy of the shadow memory (SHADOW_EN_G=true)
shadowAddr : in slv(ADDRESS_SIZE_G-1 downto 0) := (others => '0');
shadowData : out slv(DATA_SIZE_G-1 downto 0) := (others => '0');
shadowAddr : in slv(log2(SPI_NUM_CHIPS_G)+ADDRESS_SIZE_G-1 downto 0) := (others => '0');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bengineerd Should you use BitSize (instead of log2()) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it turns out neither log2 nor bitSize are correct. We want the function to return 0 when given SPI_NUM_CHIPS_G=1. Otherwise shadowAddr will be one bit larger than it was before and break builds. I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shadowData : out slv(DATA_SIZE_G-1 downto 0) := (others => '0');
-- SPI Interface
coreSclk : out sl;
coreSDin : in sl;
Expand All @@ -77,7 +77,8 @@ architecture rtl of AxiSpiMaster is
type StateType is (WAIT_AXI_TXN_S, WAIT_CYCLE_S, WAIT_CYCLE_SHADOW_S, WAIT_SPI_TXN_DONE_S, SHADOW_READ_DONE_S);


signal memData : slv(DATA_SIZE_G-1 downto 0) := (others => '0');
signal memData : slv(DATA_SIZE_G-1 downto 0) := (others => '0');
signal memAddr : slv(ADDRESS_SIZE_G+CHIP_BITS_C-1 downto 0) := (others => '0');

-- Registers
type RegType is record
Expand Down Expand Up @@ -105,26 +106,27 @@ architecture rtl of AxiSpiMaster is
begin

SHADOW_RAM_GEN : if (SHADOW_EN_G) generate
memAddr <= r.chipSel & r.wrData(DATA_SIZE_G+ADDRESS_SIZE_G-1 downto DATA_SIZE_G);
U_DualPortRam_1 : entity surf.DualPortRam
generic map (
TPD_G => TPD_G,
MEMORY_TYPE_G => SHADOW_MEM_TYPE_G,
REG_EN_G => false,
DATA_WIDTH_G => DATA_SIZE_G,
ADDR_WIDTH_G => ADDRESS_SIZE_G)
ADDR_WIDTH_G => ADDRESS_SIZE_G+CHIP_BITS_C)
port map (
clka => axiClk, -- [in]
ena => '1', -- [in]
wea => r.wrEn, -- [in]
rsta => axiRst, -- [in]
addra => r.wrData(DATA_SIZE_G+ADDRESS_SIZE_G-1 downto DATA_SIZE_G), -- [in]
dina => r.wrData(DATA_SIZE_G-1 downto 0), -- [in]
douta => memData, -- [out]
clkb => axiClk, -- [in]
enb => '1', -- [in]
rstb => axiRst, -- [in]
addrb => shadowAddr, -- [in]
doutb => shadowData); -- [out]
clka => axiClk, -- [in]
ena => '1', -- [in]
wea => r.wrEn, -- [in]
rsta => axiRst, -- [in]
addra => memAddr, -- [in]
dina => r.wrData(DATA_SIZE_G-1 downto 0), -- [in]
douta => memData, -- [out]
clkb => axiClk, -- [in]
enb => '1', -- [in]
rstb => axiRst, -- [in]
addrb => shadowAddr, -- [in]
doutb => shadowData); -- [out]

end generate SHADOW_RAM_GEN;

Expand All @@ -147,6 +149,7 @@ begin
if (ADDRESS_SIZE_G > 0) then
v.wrData(DATA_SIZE_G+ADDRESS_SIZE_G-1 downto DATA_SIZE_G) := axiReadMaster.araddr(2+ADDRESS_SIZE_G-1 downto 2); -- setup memAddr
end if;
v.chipSel := axiReadMaster.araddr(CHIP_BITS_C+ADDRESS_SIZE_G+1 downto 2+ADDRESS_SIZE_G);
elsif (MODE_G = "WO") then
axiSlaveReadResponse(v.axiReadSlave, AXI_RESP_DECERR_C);
else
Expand Down
Loading