No support for crystals other than 12MHz #1024

New issue

Closed

opened 2022-09-13 15:01:21 -05:00 by arg08 · 8 comments

arg08 commented

2022-09-13 15:01:21 -05:00

(Migrated from github.com)

Issue #748 requests support to heavily customise this startup behaviour (for example, if there is no crystal at all or it is undersirable to use it for power reasons). However, there is another class of applications that do want all the standard initialisation but simply have a crystal of different frequency (or an external clock wired to XIN). This could usefully be supported by allowing the board file to specify the crystal frequency.

There is already a symbol XOSC_MHZ defined in platform_defs.h, but this is almost entirely ignored in the setup code in clocks_init().

If the code in clocks_init() were updated to respect XOSC_MHZ, it could be overridden in the board file for boards with non-standard crystals.

One question is what setup is expected in respect of the PLLs. The minimum requirement is that the PLLs are not set up to out-of-range frequencies, causing a crash before the user program has a chance to do further setup. The ideal would be to set the desired PLL output frequencies for any crystal, but the PLL configuration tool is currently in the form of a python program not readily invoked during the build process. A compromise would be to allow the board file to define the PLL parameters, and for the code in clocks_init() to set the PLLs to x1 as a fallback if XOSC_MHZ != 12 and custom PLL parameters have not been defined.

The current runtime_init() invokes clocks_init() which is entirely hard-wired for crystal oscillator at 12MHz (and sets up PLLs assuming 12MHz base frequency), such that any board with a higher frequency crystal/external oscillator to XIN will fail before the user application gets a chance to reconfigure it. Issue #748 requests support to heavily customise this startup behaviour (for example, if there is no crystal at all or it is undersirable to use it for power reasons). However, there is another class of applications that do want all the standard initialisation but simply have a crystal of different frequency (or an external clock wired to XIN). This could usefully be supported by allowing the board file to specify the crystal frequency. There is already a symbol XOSC_MHZ defined in platform_defs.h, but this is almost entirely ignored in the setup code in clocks_init(). If the code in clocks_init() were updated to respect XOSC_MHZ, it could be overridden in the board file for boards with non-standard crystals. One question is what setup is expected in respect of the PLLs. The minimum requirement is that the PLLs are not set up to out-of-range frequencies, causing a crash before the user program has a chance to do further setup. The ideal would be to set the desired PLL output frequencies for any crystal, but the PLL configuration tool is currently in the form of a python program not readily invoked during the build process. A compromise would be to allow the board file to define the PLL parameters, and for the code in clocks_init() to set the PLLs to x1 as a fallback if XOSC_MHZ != 12 and custom PLL parameters have not been defined.

peterharperuk commented

2022-12-06 10:16:58 -06:00

(Migrated from github.com)

Is the ideal solution to this to run vcocalc.py during the build to get the PLL configuration given the crystal frequency?

I might need this explaining to me...

the code in clocks_init() to set the PLLs to x1 as a fallback

Is the ideal solution to this to run vcocalc.py during the build to get the PLL configuration given the crystal frequency? I might need this explaining to me... > the code in clocks_init() to set the PLLs to x1 as a fallback

arg08 commented

2022-12-06 10:24:11 -06:00

(Migrated from github.com)

Is the ideal solution to this to run vcocalc.py during the build to get the PLL configuration given the crystal frequency?

Ideal in one sense, but making Python a dependency of the build system may not be considered justified for just this feature.

I might need this explaining to me...

the code in clocks_init() to set the PLLs to x1 as a fallback

I simply meant that if the init code doesn't have access to sensible PLL parameters (by calculating them, or having them passed in through defines in the board file) then it can simply use the crystal frequency as clk_sys as that's always safe, and the application can sort things out later. Probably that's more sensibly done by bypassing the PLLs rather than literally setting them to a x1 configuration.

> Is the ideal solution to this to run vcocalc.py during the build to get the PLL configuration given the crystal frequency? > Ideal in one sense, but making Python a dependency of the build system may not be considered justified for just this feature. > I might need this explaining to me... > > > the code in clocks_init() to set the PLLs to x1 as a fallback I simply meant that if the init code doesn't have access to sensible PLL parameters (by calculating them, or having them passed in through defines in the board file) then it can simply use the crystal frequency as clk_sys as that's always safe, and the application can sort things out later. Probably that's more sensibly done by bypassing the PLLs rather than literally setting them to a x1 configuration.

peterharperuk commented

2023-02-13 11:33:37 -06:00

(Migrated from github.com)

@andygpz11 I might need your advice on this

andygpz11 commented

2023-02-13 11:47:04 -06:00

(Migrated from github.com)

Sure @peterharperuk
Actually I'll assign it to me.

Sure @peterharperuk Actually I'll assign it to me.

kilograham commented

2023-02-13 11:53:31 -06:00

(Migrated from github.com)

note i think the scope of this is to respect XOSC_MHZ. i would prefer not to use vcocalc during the build, but perhaps as suggested use new #defines. See if this looks neat/tidy without building a bunch of infrastructure, because we do have a separate issue for configurable startup.

andygpz11 commented

2023-02-14 06:59:44 -06:00

(Migrated from github.com)

@kilograham Having looked at the code I completely agree: Running from ROSC etc. is beyond scope of this ticket.
For this, we could & should do a better job and respect XOSC_MHZ. I think that can be done relatively easily, at least for common/standard integer type frequencies.
Perhaps we should add some static_asserts and/or warnings where the math(s) won't allow us to get an exact 48 MHz clock for the USB operations?
Not everyone will need USB connectivity but it needs to be flagged.

@kilograham Having looked at the code I completely agree: Running from ROSC etc. is beyond scope of this ticket. For this, we could & should do a better job and respect XOSC_MHZ. I think that can be done relatively easily, at least for common/standard integer type frequencies. Perhaps we should add some static_asserts and/or warnings where the math(s) won't allow us to get an exact 48 MHz clock for the USB operations? Not everyone will need USB connectivity but it needs to be flagged.

arg08 commented

2023-02-14 07:22:33 -06:00

(Migrated from github.com)

My thinking was:

Respect XOSC_MHZ
Provide some new defines to put in the board file for PLL multipliers (which the author of the board file can derive using vcocalc); if these are defined, then they get full functionality just like 12MHz boards.
If those new macros are not defined, then XOSC_MHZ=12 needs to default to the values as used at present, for backwards compatibility.
For values other than 12MHz where the user hasn't specified the multipliers, it needs to either provide a (possibly suboptimal) configuration that's valid and will boot (allowing user application code to fix things up afterwards - something not possible at present), or else compile-time failure.
My idea of a sub-optimal configuration was that if you haven't specified the PLL multipliers you just get x1 (or PLLs bypassed) so the whole system runs at XOSC_MHZ until user code does something about it. But you could be cleverer and invent some multipliers for easy cases.
But OTOH I'm leaning more to the compile-time error: someone inventing one of these board files really ought to decide what they want (which could be x1) and specify it rather than relying on cleverness in the driver that only works for easy cases.

My thinking was: - Respect XOSC_MHZ - Provide some new defines to put in the board file for PLL multipliers (which the author of the board file can derive using vcocalc); if these are defined, then they get full functionality just like 12MHz boards. - If those new macros are not defined, then XOSC_MHZ=12 needs to default to the values as used at present, for backwards compatibility. - For values other than 12MHz where the user hasn't specified the multipliers, it needs to either provide a (possibly suboptimal) configuration that's valid and will boot (allowing user application code to fix things up afterwards - something not possible at present), or else compile-time failure. - My idea of a sub-optimal configuration was that if you haven't specified the PLL multipliers you just get x1 (or PLLs bypassed) so the whole system runs at XOSC_MHZ until user code does something about it. But you could be cleverer and invent some multipliers for easy cases. - But OTOH I'm leaning more to the compile-time error: someone inventing one of these board files really ought to decide what they want (which could be x1) and specify it rather than relying on cleverness in the driver that only works for easy cases.

kilograham commented

2023-03-21 12:50:35 -05:00

(Migrated from github.com)