analogdevicesinc / msdk

Software Development Kit for Analog Device's MAX-series microcontrollers
Apache License 2.0
61 stars 82 forks source link

MXC_SYS Missing EXT_CLK Support #518

Closed jlj-ee closed 1 year ago

jlj-ee commented 1 year ago

Hi, is there a bug in the MXC_SYS implementation for the MAX32670?

In the UG, the clock tree (page 38) and GCR_CLKCTRL register description (page 66-67) both document an external clock as an available system clock option. However, the implementation in sys_me15.c has support for EXTCLK removed:

In MXC_SYS_ClockSourceEnable (returns an error)

    case MXC_SYS_CLOCK_EXTCLK:
        // MXC_GCR->clkctrl |= MXC_F_GCR_CLKCTRL_EXTCLK_EN;
        // return MXC_SYS_Clock_Timeout(MXC_F_GCR_CLKCTRL_EXTCLK_RDY);
        return E_NOT_SUPPORTED;
        break;

In MXC_SYS_ClockSourceDisable (silent failure)

    case MXC_SYS_CLOCK_EXTCLK:
        // MXC_GCR->clkctrl &= ~MXC_F_GCR_CLKCTRL_EXTCLK_EN;
        break;

In MXC_SYS_ClockSelect (silent failure)

    case MXC_SYS_CLOCK_EXTCLK:
        // Enable HIRC clock
        // if(!(MXC_GCR->clkctrl & MXC_F_GCR_CLKCTRL_EXTCLK_EN)) {
        //     MXC_GCR->clkctrl |=MXC_F_GCR_CLKCTRL_EXTCLK_EN;

        //     // Check if HIRC clock is ready
        //     if (MXC_SYS_Clock_Timeout(MXC_F_GCR_CLKCTRL_EXTCLK_RDY) != E_NO_ERROR) {
        //         return E_TIME_OUT;
        //     }
        // }

        // Set HIRC clock as System Clock
        // MXC_SETFIELD(MXC_GCR->clkctrl, MXC_F_GCR_CLKCTRL_SYSCLK_SEL, MXC_S_GCR_CLKCTRL_SYSCLK_SEL_EXTCLK);

        break;

I will be populating the EXT_CLK footprint on my EV kit to test this disabled code.

(Also, MXC_SYS does not appear in the peripheral documentation.)

Jake-Carter commented 1 year ago

Hi @jlj-ee, thanks for reporting this. It looks like this has been copy/pasted into all the mxc_sys drivers for all micros...

There is not a EXT_CLK enable bit. Instead, the drivers are expected to...

  1. Configure the associated GPIO's alternate function to select it as the EXT_CLK input.

  2. Monitor the GCR extclk_rdy bit...

image

  1. ... and then select the external clock as input with the GCR sysclk_sel field.

image

Unfortunately the original implementation in our mxc_sys drivers don't do this... The MAX32670EVKIT exposes the EXT_CLK input pin (P0.12) so we can test a new implementation easily.

I've opened #521 where I will be pushing the updated sys drivers. I also fixed the MXC_SYS doxygen issues you pointed out.

jlj-ee commented 1 year ago

Thanks @Jake-Carter for looking into this! That sequence for establishing the EXT_CLK input makes sense. I am also happy to help test on my EV Kit.

Jake-Carter commented 1 year ago

Hi @jlj-ee, I added EXT_CLK support for the MAX32670 and tested it at 2Mhz. I don't have a clean enough connection to test at higher speeds, but it looks like Hello_World is working as expected off of the external input.

See the EXT_CLK example on my fork.

I'll be porting this to the other micros as well before merging the PR.

jlj-ee commented 1 year ago

Hi @Jake-Carter, I got a chance to verify the example from your fork on my EVKit and it looks great! When I have some more time I will look at additional speeds. Thanks for getting this fix in!