I think I found a bug in the fake_pll implementation in cog_ctr.v?

andrewsi

So in cog_ctr.v, there's a simulated PLL at the end using this code:

always @(posedge clk_pll)
if (~&ctr[30:28] && |ctr[27:26])
    pll_fake <= pll_fake + {4'b0, frq};

wire [7:0] pll_taps = pll_fake[35:28];
assign pll          = pll_taps[~ctr[25:23]];

The first bit is checking the 5 CTRMODE bits in the CTRA or CTRB registers, and I think what it's trying to do is ensure that the 3 MSBs are all zeroes and that at least one of the 2 LSBs is set, in other words the whole expression is true (and thus the pll is enabled) when the 5 bits are modes: 00001, 00010, 00011, but none of the others. This would make sense since these are the three PLL enabled counter modes.

However, I think the "~&ctr[30:28]" should actually be "~|ctr[30:28]" instead, shouldn't it? The present form is effectively a NAND of bits 30-28, so that part is true any time that any of those bits is zero, which would enable the fake PLL in all sorts of counter modes where it really shouldn't be. I think it should be the second form (NOR) instead, which would allow the fake PLL to be enabled only when all three bits are zero and in none of the other modes.

Have I made a fundamental error here (which would not surprise me in the slightest), or have I actually found a bug?

andrewsi

Whoops, meant to post this in the subforum. I'll repost there.