Bug in SDK system_LPC802.c returning SystemCoreClock?

[wkh]

Original code in system_LPC802.c:

void SystemCoreClockUpdate (void) {

  switch (SYSCON->MAINCLKSEL & 0x03) {
    case 0:                                       /* Free running oscillator (FRO) */
      SystemCoreClock = g_Fro_Osc_Freq;
      break;
    case 1:                                       /* System oscillator */
      SystemCoreClock = CLK_OSC_IN;
      break;
    case 2:                                       /* low power oscillator */
      SystemCoreClock = CLK_OSC_LP;
      break;
    case 3:                                       /* Free running oscillator (FRO) / 2 */
      SystemCoreClock = (g_Fro_Osc_Freq >> 1);
      break;
  }

  SystemCoreClock /= SYSCON->SYSAHBCLKDIV;
}

... returns the wrong frequency?

I have replaced  g_Fro_Osc_Freq  with CLOCK_GetFroFreq() or  (g_Fro_Osc_Freq   / 2).

Correct?

[1234567890]

Hi,

because of the binary system bit shifting by 1 to the right side (value >> 1) is equal to (value / 2).

[wkh]

I want to explain, that the global value g_Fro_Osc_Freq is setting the wrong SystemCoreClock  value (called main clock in the chip user manual, too) :

In fsl_clock.c the required division by 2 happens correctly (user manual explains that the system clock/main clock is half of the value used for setting the free running oscillator clock, e.g. 30MHz FRO results into 15MHz main clock or system clock). g_Fro_Osc_Freq is the value to set the FRO. To update the  SystemCoreClock  value requires to divide g_Fro_Osc_Freq  by 2. So I mean the SystemCoreClockUpdate  has a bug.

/*! brief  Return Frequency of FRO.
 *  return Frequency of FRO.
 */
uint32_t CLOCK_GetFroFreq(void)
{
    return g_Fro_Osc_Freq / 2;
}

So the correct code should be finally in system_LCP802.c ... ...

void SystemCoreClockUpdate (void) {

  switch (SYSCON->MAINCLKSEL & 0x03) {
    case 0:                                       /* Free running oscillator (FRO) */
      SystemCoreClock = g_Fro_Osc_Freq / 2;
      break;
    case 1:                                       /* System oscillator */
      SystemCoreClock = CLK_OSC_IN;
      break;
    case 2:                                       /* low power oscillator */
      SystemCoreClock = CLK_OSC_LP;
      break;
    case 3:                                       /* Free running oscillator (FRO) / 2 */
      SystemCoreClock = (g_Fro_Osc_Freq >> 2);   // or g_Fro_Osc_Freq / 4
      break;
  }

  SystemCoreClock /= SYSCON->SYSAHBCLKDIV;
}

I only want to get the confirmation and if my meaning is correct somebody from NXP will fix this in the SDK (or fsl) :-)

[1234567890]

Now I understand. I don't know what is correct, but it's not consistent.

And the manual isn't very clear.

Page 54 and 55 are the same.

On page 57 "main_clock_pre_pll" is the only place in the manual where this word is used.

And here you can see that FRO is selectable. And FRO is the oscillator frequency, I think. But on page 52 is this:

So this looks like fixed devision, without a choice for the user.

I'm confused as well.

[wkh]

Finally vendor independent ARM defines in the core include(e.g. core_cm0plus.h) files the following function ...

uint32_t SysTick_Config(uint32_t ticks)

Al last the following combination shall work independent of any vendor specific hardware abstraction layer to enable the

SysTick interrupt:

    SystemCoreClockUpdate();                          //  <-- implementation by vendor

    SysTick_Config(SystemCoreClock /1000);     // <--- implemented by ARM - SysTick_Config

With the current buggy implementation of SystemCoreClockUpdate the systick interrupt will be twice slow.

Additional the SystemCoreClockUpdate()  shall be used to verify that any change of the system core clock is done correctly by an user.

Common implementation of other vendors is to read again all clock registers. In older implementation this has been done in the SystemCoreClockUpdate() as well by NXP. With the new fsl library (SDK part) the SystemCoreClockUpdate() is now based by setting of the global

variable g_Fro_Osc_Freq  and confusing hard coded C defines. Finally changing the defines to get a different startup frequency from e.g. 12MHz to 15Mhz requires to change on 2 places (C defines and to exchange the i a C function a hard coded fsl call with system clock/fro value value 15Mhz in the function name. Not very comfortable.

[ZhangJennie]

Karlheinz Wuersch

This issue will be fixed in SDK2.7.0

Have a great day,
Jun Zhang

-------------------------------------------------------------------------------
Note:
- If this post answers your question, please click the "Mark Correct" button. Thank you!

- We are following threads for 7 weeks after the last post, later replies are ignored
Please open a new thread and refer to the closed one, if you have a related question at a later point in time.
-------------------------------------------------------------------------------