[dv,jtag] Change JTAG clock frequency to 24MHz

[sha-ron]

The current configured frequency of 50MHz is too fast and might not be supported by silicin.

[sha-ron]

he current configured frequency of 50MHz is too fast and might not be supported by silicon.

[a-will]

Not that it necessarily matters for this round, but... This merely changes the defaults for a parameter. Shouldn't the value for earlgrey be set in some using module or elsewhere via the DV environment?

[a-will]

For example, at these sites:

opentitan/hw/top_earlgrey/dv/env/chip_if.sv

Line 437 in 007358e

jtag_if jtag_if();

opentitan/hw/top_earlgrey/dv/env/chip_if.sv

Line 457 in 007358e

jtag_if flash_ctrl_jtag_if();

However, if we end up setting the value in any sequences, then this would get overridden anyway. It looks like we only do that in block-level tests right now, though.

[sha-ron]

Yes, I agree it might be good to do it differently at later stage. For now I tried to minimize the changes as much as possible.

[rswarbrick]

What situation are you using this DV code in? Are you really sure that the numbers you change here have the effect you expect? For example, in the rv_dm block tests, the frequency is randomised to be between 1MHz and 10MHz. (See rv_dm_base_vseq.sv)

[sha-ron]

It failed during gate-level simulation since 50MHz frequency is too fast and is not supported. 24MHz is supported and works fine.

[rswarbrick]

Ah, I see. Presumably you are instantiating the interface in gate-level simulation with some code that configures it. The easiest approach is probably to choose a "GLS-suitable frequency" there.

[sha-ron]

I don't have special tests, I am using existing tests like rom_raw_unlock in which the JTAG frequency is set by this interface. But maybe indeed it will be simpler to do the change under `ifdef GATE_LEVEL?

[rswarbrick]

Ah right, now I think I understand. If I've followed this correctly, the instantiation of jtag_if comes from chip_if.sv, line 437.

For some bizarre reason, the original author of this interface made it "active", so that it drives its own TCK signal based on a configured frequency. Honestly, I think this is mad! But changing that is not a problem for today :-)

Looking a bit further (grepping for jtag_if!), I see that in the DV environment, it gets wired up to a UVM agent in chip_if.sv, line 985. Oddly, that agent doesn't configure the JTAG frequency itself. At block level, the lc_ctrl and rv_dm tests do configure it (in the env for lc_ctrl and the base vseq for rv_dm).

I think we should probably be a little more explicit in DV land ("my chip JTAG frequency is XYZ"), but that's not really a problem for today either! But it would be nice to keep the fast-ish JTAG frequency in DV simulations: they are painfully slow already! If you can stick it under an ifdef GATE_LEVEL that would be perfect. Possibly with a TODO comment reminding us to clean things up!

[sha-ron]

Yes, I agree the configuration should be done through a UVM agent. For now I added it under `ifdef GATE_LEVEL with a TODO comment.

[rswarbrick]

Thanks for this. I've just opened an issue about the fact this needs sorting out and have force-pushed to this PR with a link to it. Merging without waiting for CI to run again (the diff shows I've just added a token to a comment!)

[sha-ron]

Thank you for your help @rswarbrick .