TM4C129ENCPDT: Suggestions for next version of TivaWare

Item 1:

TivaWare supplies a function TimerConfigure() with variants ROM_TimerConfigure() and MAP_TimerConfigure().

This is a great function but if you want to "split" the timer (e.g., use a 32-bit timer as two 16-bit timers) you have to configure both halves in the same call. Otherwise, if configuring one half timer, say in one source code module, and then configuring the other half in another source code module for another use, the later configuration will undo part or all of the earlier configuration.

To work around this and allow my software to configure half-timers independently, I made a modified copy of TimerConfigure() called TimerConfigureHalf(). This function is used similarly to TimerConfigure() except that it is not necessary to specify TIMER_CFG_SPLIT_PAIR because that is implied by the function itself.

I recommend mentioning in the documentation block above TimerConfigure() that it affects both halves of the timer.

The following is the TimerConfigureHalf() function I am using on TM4C129ENCPDT and TM4C123AH6PM. It works fine for me, but I have only tested it as far as it concerns my application, so please review carefully!!

/*! \brief Configures a half-timer when operating as a split pair.

	Use this instead of TivaWare-supplied TimerConfigure() because that
	function forces configuration of both halves of the timer in the same
	call. Using two calls to that function to configure each half-timer
	separately results in the earlier configuration being clobbered by the
	later one.

	The half-timer is disabled before being configured and is left in the
	disabled state.

	\param[in] ui32Base Base address for the timer peripheral, which is one of
	the following, subject to availability on the TM4C part in use:
	TIMER0_BASE, TIMER1_BASE, TIMER2_BASE, TIMER3_BASE,
	TIMER4_BASE, TIMER5_BASE, TIMER6_BASE, TIMER7_BASE,
	WTIMER0_BASE, WTIMER1_BASE, WTIMER2_BASE,
	WTIMER3_BASE, WTIMER4_BASE, WTIMER5_BASE.

	\param[in] ui32Timer Which half-timer to configure, which is one of
	TIMER_A, TIMER_B.

	\param[in] ui32Config Configuration mask. If configuring TIMER_A, one of
	the following: TIMER_CFG_A_ONE_SHOT, TIMER_CFG_A_ONE_SHOT_UP,
	TIMER_CFG_A_PERIODIC, TIMER_CFG_A_PERIODIC_UP, TIMER_CFG_A_CAP_COUNT,
	TIMER_CFG_A_CAP_COUNT_UP, TIMER_CFG_A_CAP_TIME, TIMER_CFG_A_CAP_TIME_UP,
	TIMER_CFG_A_PWM, optionally bitwise ORed with one of
	TIMER_CFG_A_ACT_TOINTD, TIMER_CFG_A_ACT_NONE, TIMER_CFG_A_ACT_TOGGLE,
	TIMER_CFG_A_ACT_SETTO, TIMER_CFG_A_ACT_CLRTO, TIMER_CFG_A_ACT_SETTOGTO,
	TIMER_CFG_A_ACT_CLRTOGTO, TIMER_CFG_A_ACT_SETCLRTO, or
	TIMER_CFG_A_ACT_CLRSETTO. Similarly if configuring TIMER_B, substitute the
	B variant of the above.
*/
void TimerConfigureHalf(uint32_t ui32Base, uint32_t ui32Timer, uint32_t ui32Config)
{
	ASSERT(_TimerBaseValid(ui32Base));
	ASSERT((ui32Timer == TIMER_A) || (ui32Timer == TIMER_B));

	// This function always configures split pairs
	ui32Config |= TIMER_CFG_SPLIT_PAIR;

	if (ui32Timer == TIMER_A) {
		ASSERT(
			((ui32Config & 0x000000ff) == TIMER_CFG_A_ONE_SHOT) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_ONE_SHOT_UP) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_PERIODIC) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_PERIODIC_UP) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_CAP_COUNT) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_CAP_COUNT_UP) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_CAP_TIME) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_CAP_TIME_UP) ||
			((ui32Config & 0x000000ff) == TIMER_CFG_A_PWM)
		);

		// Disable the timer
		HWREG(ui32Base + TIMER_O_CTL) &= ~(TIMER_CTL_TAEN);

		// Set the global part of the timer configuration to split pair
		HWREG(ui32Base + TIMER_O_CFG) = TIMER_CFG_SPLIT_PAIR >> 24;

		// Set the configuration of the half-timer and set the TxPWMIE bit
		if (CLASS_IS_TM4C129) {
			HWREG(ui32Base + TIMER_O_TAMR) = (
				((ui32Config & 0x000f0000) >> 4) |
				(ui32Config & 0xff) |
				TIMER_TAMR_TAPWMIE
			);
		}
		else {
			HWREG(ui32Base + TIMER_O_TAMR) = (
				(ui32Config & 0xff) |
				TIMER_TAMR_TAPWMIE
			);
		}
	}
	else if (ui32Timer == TIMER_B) {
		ASSERT(
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_ONE_SHOT) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_ONE_SHOT_UP) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_PERIODIC) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_PERIODIC_UP) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_CAP_COUNT) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_CAP_COUNT_UP) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_CAP_TIME) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_CAP_TIME_UP) ||
			((ui32Config & 0x0000ff00) == TIMER_CFG_B_PWM)
		);

		// Disable the timer
		HWREG(ui32Base + TIMER_O_CTL) &= ~(TIMER_CTL_TBEN);

		// Set the global part of the timer configuration to split pair
		HWREG(ui32Base + TIMER_O_CFG) = TIMER_CFG_SPLIT_PAIR >> 24;

		// Set the configuration of the half-timer and set the TxPWMIE bit
		if (CLASS_IS_TM4C129) {
			HWREG(ui32Base + TIMER_O_TBMR) = (
				((ui32Config & 0x00f00000) >> 8) |
				((ui32Config >> 8) & 0xff) |
				TIMER_TBMR_TBPWMIE
			);
		}
		else {
			HWREG(ui32Base + TIMER_O_TBMR) = (
				((ui32Config >> 8) & 0xff) |
				TIMER_TBMR_TBPWMIE
			);
		}
	}
} // TimerConfigureHalf

Item 2:

We use the MAP_-prefixed functions. It is my understanding that most of those map to the TM4C12x's ROM, except in cases where a ROM function has been superseded by a newer version in TivaWare, in which case the MAP_-prefixed version will map to the un-prefixed compiled-into-Flash version.

I noticed however that some functions seem to lack a MAP_-prefixed version. For example, TivaWare does not provide a MAP_TimerUpdateMode(). I don't know if that's intentional but it causes a couple of minor problems: (1) I can't remember which functions lack a MAP_-prefixed version, so it results in an extra compile-error-repair-compile cycle; also (2) it looks like a mistake in my code, since it looks as if I forgot the MAP_ prefix on one function call.

Wherever I've discovered a missing MAP_-prefixed version of a function, I've gone ahead and added one. It's just a define that resolves to the non-prefixed version but it saves me some headache. I would suggest to give every TivaWare function a MAP_-prefixed version.

Item 3:

For reasons I don't remember, I needed my program code to query whether a timer is currently enabled or disabled. So I added a TimerIsEnabled() function:

/*! \brief Checks if Timer peripheral is enabled

	\param[in] ui32Base Base address of the timer peripheral (one of the
	TIMER*_BASE defines)

	\param[in] ui32Timer Which timer(s) within the peripheral to enable (one of
	\b TIMER_A, \b TIMER_B, or \b TIMER_BOTH)

	\returns true if the timer is enabled, false if disabled
*/
bool TimerIsEnabled(uint32_t ui32Base, uint32_t ui32Timer)
{
	uint32_t TmrOCtl;
	uint32_t Expected;

	// Check the arguments
	ASSERT(_TimerBaseValid(ui32Base));
	ASSERT((ui32Timer == TIMER_A) || (ui32Timer == TIMER_B) || (ui32Timer == TIMER_BOTH));

	// Determine if the specified timer(s) is/are enabled
	TmrOCtl = HWREG(ui32Base + TIMER_O_CTL);
	Expected = ui32Timer & (TIMER_CTL_TAEN | TIMER_CTL_TBEN);

	return ((TmrOCtl & Expected) == Expected) ? true : false;
} // TimerIsEnabled

Item 4:

GPIOPadConfigSet() contains several for-loops to configure registers that only appear on TM4E111 and later device classes. The comments near those for-loops state that it's a harmless write on older devices.

If program code only calls GPIOPadConfigSet() at initialization, then it's not a big deal, but my code sometimes changes pin configurations on-the-fly for various reasons. To increase performance, I wrapped those for-loops with checks for CLASS_IS_TM4C129 to avoid executing that code needlessly.

Hello twelve12pm,

Thanks a lot for these suggestions!

Regarding Item #2, I think what happened is new functions which were added after the ROM was frozen didn't get MAP_ prefixes, likely because it was forgotten to add. I say this because I didn't even think to add MAP_ prefixes to the three new API's we have added. :) Will put it on the to do list.

Regarding Item #4: That seems reasonable, and I don't see an issue with adding that. Will investigate further.

I like the the idea of the TimerConfigureHalf and will look into that more. Either way we can update the documentation regarding the need to configure both Timers at once.

Will need look into #3 further before commenting.

Ralph Jacobi said:

Hello twelve12pm,

Thanks a lot for these suggestions!

Regarding Item #2, I think what happened is new functions which were added after the ROM was frozen didn't get MAP_ prefixes, likely because it was forgotten to add. I say this because I didn't even think to add MAP_ prefixes to the three new API's we have added. :) Will put it on the to do list.

Regarding Item #4: That seems reasonable, and I don't see an issue with adding that. Will investigate further.

I like the the idea of the TimerConfigureHalf and will look into that more. Either way we can update the documentation regarding the need to configure both Timers at once.

Will need look into #3 further before commenting.

Sure thing. Let me know if you have any questions or need more information. If I think of additional things I'll post them here.

Thanks

Hello twelve12pm,

#2 and #4 have been implemented (well, #4 needs to be test build as I just did that change, but it will be!)

Speaking of #4, it looks like that was never intended to be in TivaWare but it wasn't excluded from the public build properly. Whoops.

For #3, I am not sure there is a lot of value to adding that API, but I am going to run it by Bob/Charles in our next review meeting.

We will review and discuss #1 then as well.

As a side effect of #2, I also noticed many of our TivaWare examples use ROM_ instead of MAP_ calls, so going to try and clean that up too.

Ralph Jacobi said:

Speaking of #4, it looks like that was never intended to be in TivaWare but it wasn't excluded from the public build properly. Whoops.

Are you sure? I think these loops apply to the TM4C129 devices.

Ralph Jacobi said:

For #1, we were on the fence with this as we can see the value, but so far we also haven't had any real cases of customer running into issues due to it.

Except me. :-)

Ralph Jacobi said:

The effort you put into making the function and then sharing it is very much appreciated though. What we have decided for now is that it won't be implemented in the upcoming TivaWare release, but we will keep the post bookmarked in our resource list. If we end up finding that we were wrong about the assessment of it's utility, then we can consider it again in the future.

Fair enough. Thanks for considering it.

Ralph Jacobi said:

Really appreciate again that you raised #2 and #4 as those are solid changes we can make, and ones we would have entirely missed without you calling them out. I ended up adding 24 more MAP_ functions in total.