WIZwiki-W7500 SystemCoreClock at 20MHz instead of 48MHz

[Willem23]

A quick first test with the new WIZwiki-W7500 platform shows that the clockfrequency is 20MHz, which is less than half the advertised value of 48MHz. Turns out that this is due to the fact that the PLL freq calculation register is not initialised and uses its reset value of 0x050200 instead. That means the PLL config values are: M=5, N=2, OD=1. So the SystemCoreClock = 8MHz * M / N * 1 / OD = 20 MHz

This bug should be fixed in the W7500 mbed source code, probably in system_W7500x.c, for example by adding:
CRG->PLL_FCR = 0x060100;

I also noted that the internal Xtal (8MHz) is used instead of the (mounted) external Xtal (8 MHz). Any reason for that?

[0xc0170]

cc @hjjeon0608

[hjjeon0608]

We set core clock as 20MHz on purpose not bug.
Do I change the core clock to maximum frequency(48MHz)??

We mounted external Xtal for user to test. We think it is enough to use internal Xtal.
And do I change also???

[Willem23]

I think most users would prefer the maximum advertised clockspeed. So 48 MHz should be the default setting for the mbed libary. The user code could change that back again if desired to save power consumption, but that will be more difficult since it also has effects on other components in the mbed lib. I noticed for example that the wait() method does no longer provide correct delays when you change the clockspeed in the user code. I guess the user code would then also need to reinitialise the wait() timer to account for the new clockspeed.

I have not tested the stability of the internal oscillator. It may be fine and then there is no need for a change to the external xtal. Although why not use it when it is mounted..

I did notice however that there is a hardcoded reference to the 8MHz clock in parts of the code. This may be a problem when you ever change the xtal or clocksource later.
For example in cmsis/../W7500x_uart.c :

uint32_t UART_Init(UART_TypeDef UARTx, UART_InitTypeDef UART_InitStruct)
{
...
// Set baudrate
CRG->UARTCLK_SSR = CRG_UARTCLK_SSR_RCLK; // Set UART Clock using internal Osc (8MHz)
uartclock = (8000000UL) / (1 << CRG->UARTCLK_PVSR);
...
}

I think the hardcode value should at least be changed into the #define found in system_W7500x.h
#define INTERN_XTAL (8000000UL)

[Sissors]

In general it is good if the init functions can handle different clock speeds (so no hardcoded values), and what might be nice is just in your clock setup file to have several options. I think the default should be 48MHz, especially since that is what users expect. But you can also add the 20MHz setup as alternative (assuming it has an advantage, well power consumption obviously) so someone who wants to use that option can easily modify the file to select that.

For example this is for the Freescale KL25z: https://github.com/mbedmicro/mbed/blob/master/libraries/mbed/targets/cmsis/TARGET_Freescale/TARGET_KLXX/TARGET_KL25Z/system_MKL25Z4.c. A user can easily choice between different clocking options.

[0xc0170]

@Sissors +1 for the first paragraph.

@Willem23 Thanks for raising this. Those hardcoded value shall be removed.

[MaryJane136]

I would like to know - gracefully - when it will be realized? Thank you.

[ciarmcom]

ARM Internal Ref: IOTMORF-250