initClocks in MSP430's wiring.c doesn't initialize BCSCTL2 properly and double-calls enableXtal

[robertinant]

From @barawn on June 3, 2016 21:11

The initClocks function for MSP430s based on Basic Clock Module+ (with BC2) doesn't really initialize BCSCTL2 properly. All it does in initClocks() is: /* SMCLK = DCO / DIVS = nMHz */ BCSCTL2 &= ~(DIVS_0); enableXtal(); In fact, that code does nothing, because DIVS_0 is 0, so ~(DIVS_0) is 0xFF, and BCSCTL2 is an 8-bit register, so bitwise-anding it with 0xFF does nothing. I think what is desired here was actually "BCSCTL2 &= ~(DIVS_3)", namely, clearing all of the DIVS bits.

By the user's guide, BCSCTL2 should start up at 0, so this shouldn't be a problem, but it would be a lot more readable to explicitly replace that with

/* SMCLK = DCO / DIVS = nMHz */ BCSCTL2 = SELM_0 | DIVM_0 | DIVS_0;

so that it's obvious exactly what the SMCLK and MCLK are being set to.

I removed enableXtal() there because it's actually called at the end of that function anyway. enableXtal takes a huge amount of time if the crystal's not present, so calling it twice has a big downside.

Copied from original issue: energia/Energia#888

[robertinant]

From @rei-vilo on June 7, 2016 14:15

Which MSP430s do you have in mind? There is quite large a range available...

[robertinant]

From @StefanSch on June 7, 2016 21:17

i can confirm this issues - it applies for the G2xxxx devices (devices with Basic clock module) just committed the fix.

• enable Xtal is called twice (again at end of the functions for all devices)
  • BCSCTL2 setting has no effect so removed both lines