It is not obvious what unsetting and setting CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS in clock_configure() achieves

[kahara]

(Likely reason being that I don't understand what's going on, but would still appreciate if someone who is knowledgeable enough, would do the explaining.)

The clock_configure() function can be used to set up any of the clock. In that function there's a pair of hw_{clear,set}_bits() calls that write CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS to the clock's CTRL register.

But the value of CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS, 0x00000800, does not make sense with clk_sys for example, because in that clock's CTRL register the bits 31:8 are marked as "Reserved" (2.15.7. List of Registers).

[lurch]

I'm afraid I dunno the answer to your question, but in the meantime I can tell you that CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS relates to this part of the datasheet:

Ahhh.... perhaps the fact that it's explicitly using CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS is in fact a red-herring, and the code is simply relying on the fact that every clock has its ENABLE bit at the same bit-position as CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS 🙂 (which is a safe assumption to make, as all the clocks registers are generated programatically).

EDIT: And writing to a non-existent part of a register probably has no effect? So it's probably easier to just "write to the clock's ENABLE field (regardless of whether it exists or not)" than it is to "check if the clock has an ENABLE field, and then write to it" ? 🤷‍♂️

[kilograham]

EDIT: And writing to a non-existent part of a register probably has no effect? So it's probably easier to just "write to the clock's ENABLE field (regardless of whether it exists or not)" than it is to "check if the clock has an ENABLE field, and then write to it" ? man_shrugging

Which I think was the original question :-)

[kahara]

Thank you, I'll add that as a comment next to those calls and make a PR.

[Wren6991]

Yes that code assumes that that bit has no effect on the always-on clocks clk_sys and clk_ref, where it is reserved. Not a good assumption to make in general, but it is true for this hardware.

It is good to call this out explicitly, thank you for the PR.

[Wren6991]

#414 is merged