iCE40 BRAM errata workaround silently disappears once a clock domain named "sync" is defined

[cbiffle]

For whatever reason I manage to tickle the iCE40 BRAM startup erratum about every three years.

In this case, I'm evaluating Amaranth and brought up the iCE40 PLL for the first time. I wrote something derived from @adamgreig's wonderful examples (but updated to work with current Amaranth) and produced code that looked like this:

        cd_sync = ClockDomain("sync")
        m.domains += cd_sync

        platform.add_clock_constraint(cd_sync.clk, 72e6)

        m.submodules.pll = Instance(
            "SB_PLL40_CORE",
            p_FEEDBACK_PATH = "SIMPLE",
            p_DIVR = 0,
            p_DIVF = 47,
            p_DIVQ = 3,
            p_FILTER_RANGE = 1,

            i_REFERENCECLK = platform.request("clk12", dir = "-").io,
            i_RESETB = 1,
            o_PLLOUTGLOBAL = cd_sync.clk,
        )

Figured that was about the simplest thing that could work. However, the design became intermittently flaky. I recognized the flaky behavior from a past life and began digging into the Amaranth iCE40 platform code.

I find the code difficult to follow, but from what I can tell, this erratum fix gets disabled as soon as I add a domain called sync. (My conclusion there could be wrong -- the docs in this area are thin and the inheritance subtle.)

This behavior seems surprising, and I feel like it should at least be exposed in documentation, if not changed.

[whitequark]

I find the code difficult to follow, but from what I can tell, this erratum fix gets disabled as soon as I add a domain called sync.

That is accurate and working as designed.

This behavior seems surprising, and I feel like it should at least be exposed in documentation, if not changed.

It should probably be documented. That said, if you create a domain yourself you implicitly acknowledge that you have informed yourself of all relevant information and take all responsibility, so I don't see this changing.

[cbiffle]

Alright! Surfacing it in the docs for the platform would probably resolve my concern, since that was the first place I looked when I was starting to debug it.

[whitequark]

On second thought, maybe we should make it a default-on warning that is silenced by

1. using a default sync domain;
2. setting platform.ice40_bram_errata_handled = True

(We probably should only display it if we find any memories in the design...)