artiq icon indicating copy to clipboard operation
artiq copied to clipboard

urukul: consistent clocking defaults

Open airwoodix opened this issue 4 years ago • 0 comments

ARTIQ Feature Request

Problems this request addresses

  1. the urukul coredevice driver defaults to clk_sel = 0 but refclk = 125e6, which is strange since clk_sel = 0 is always the on-board 100 MHz XO;

  2. the AD9910 coredevice driver defaults to pll_n = 40 which produces an invalid sysclk (greater than 1 GHz) when using the Urukul defaults (refclk = 125e6, clk_div = 0 (divide by 4));

  3. When using the artiq_ddb_template, the following JSON snippet leads to a working but inconsistent driver configuration:

        {
            "type": "urukul",
            "dds": "ad9910",
            "ports": [n, m],
            "clk_sel": 0
        }

artiq_ddb_template most of the time writes refclk = 125e6 and pll_n = 32 [ref] so, indeed, sysclk = 1e9 when clk_div = 0 but this is the same issue as point 1;

  1. The current implementation of artiq_ddb_template requires indicating both refclk and pll_n to make the clk_sel = 0 example consistent and working. Indicating refclk is redundant and error-prone (#1474).

Describe the solution you'd like

  1. Use consistent defaults in AD9910/Urukul coredevice drivers. For example clk_sel = 0, refclk = 100e6, clk_div = 0, pll_n = 40 sounds the safest to me, since the 100 MHz XO is present on all boards and doesn't require extra hardware configuration;

  2. Make artiq_ddb_template set refclk = 100e6 and pll_n = 40 when clk_sel = 0 for AD9910;

  3. Alternatively (to b), make artiq_ddb_template fail when refclk (and pll_n?) is not present. This might make most sense since it would force users to document what the external reference frequency is supposed to be when clk_sel = 2.

Additional context

  1. It is debatable whether more logic should be added in artiq_ddb_template. JSON files are relatively low-level tools and one may consider that they need to stay this way. My experience is however that I tend to want to rely on artiq_ddb_template to catch obvious errors in the coredevice drivers configuration part of the JSON file.

I'd be happy to prepare PRs for such patches if there's a consensus.

airwoodix avatar Oct 19 '20 10:10 airwoodix