• State
  • Locked Locked
  • Replies 5 replies
  • Subscribers 31 subscribers
  • Views 1763 views
  • Users 0 members are here
Support feedback
Options
Options
Similar topics

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Tiva ADCClockConfigSet parameter validation 2.1.0.12573

Tim Cooper1
Intellectual 980 points

The function validation of the input parameter ui32ClockDiv appears to be incorrect.  The function prototype is:

ADCClockConfigSet(uint32_t ui32Base, uint32_t ui32Config,
                  uint32_t ui32ClockDiv)

The function sets the clock divisor (a 6 bit value) in ADC_O_CC to (ui32Config - 1) << ADC_CC_CLKDIV_S, which is correct, since the field occurs in bits (9-4) of the register.  However, the function validates the input parameter using:

    ASSERT(((ui32ClockDiv - 1) & ~ADC_CC_CLKDIV_M) == 0);

where ADC_CC_CLKDIV_M is 0x3f0 (that is, the shifted mask value).  This test is not correct;  certain valid input divisor values (17, 33) will be rejected, while other invalid values may be accepted.  The test should be:

    ASSERT((((ui32ClockDiv - 1) << ADC_CC_CLKDIV_S) & ~ADC_CC_CLKDIV_M) == 0);

  • 0
    TI__Guru**** 244380 points

    Hello Tim,

    Agreed. However for the values of the divider is based on VCO/16 or VCO/32.

    For the VCO's of 480 and 320MHz, the values will work out to be fine and thus it will make through the ASSERT.

    I will still go ahead and file a bug on the software for correction.

    Regards

    Amit

  • 0 in reply to Amit Ashara
    Intellectual 430 points
    Hello. I am using the latest (AFAIK) revision (2.1.1.71) and the assertion is now:

    ASSERT(((ui32ClockDiv - 1) & (ADC_CC_CLKDIV_M >> ADC_CC_CLKDIV_S)) == 0);

    I am calling the function with a ui32ClockDiv equal to 60 and the assertion is false.

    Looking at the values of the macros I think there should be a '~' to negate the mask i.e.

    ASSERT(((ui32ClockDiv - 1) & ~(ADC_CC_CLKDIV_M >> ADC_CC_CLKDIV_S)) == 0);

    otherwise it will still fail with valid divisors and proceed with invalid ones.
  • 0 in reply to Elder Costa
    TI__Guru**** 244380 points
    Hello Elder,

    It should be the following since the value should not exceed 63

    ASSERT((ui32ClockDiv - 1) <= (ADC_CC_CLKDIV_M >> ADC_CC_CLKDIV_S));

    Also the ASSERT for base address is not correct as well.

    Thanks and Regards
    Amit
  • 0 in reply to Amit Ashara
    Intellectual 430 points
    Hello Amit. Thank you for the quick response. I like it better as it is much simpler and straightforward.

    However this issue makes me wonder how thorough is the code review process for Tivaware as the latest version release notes (spmu299b, section 1.4.2) states the ASSERT has been fixed.
  • 0 in reply to Elder Costa
    TI__Guru**** 244380 points
    Hello Elder

    Indeed this is a problem in the code review and this has been added to the report. The original issue was it was not looking at the correct set of bits and when we "fixed" it the ASSERT was not the true condition.

    Regards
    Amit