mcux-sdk: enet: fsl_enet: avoid possible race condition on timestamp

[theadib]

Only correct second value if the nanosecond has reasonably value.

Prerequisites

• [X ] I have checked latest main branch and the issue still exists.
• I did not see it is stated as known-issue in release notes.
• No similar GitHub issue is related to this change.
• My code follows the commit guidelines of this project.
• I have performed a self-review of my own code.
• My changes generate no new warnings.
• I have added tests that prove my fix is effective or that my feature works.

Describe the pull request
The K64 has only nanoseconds part of the second timer.
Second and nanosecond can not be captured at the same time.

If the nanoseconds are close to the overflow care should be taken.

The second correction should be applied if

• after the copy of the second value and
• before the capture of the nanoseconds part
  an overflow of the nanoseconds happened.
  The nanosecond value is small and the second value must be incremented.

if the overflow happend

• after the capture of the nanoseconds
• but early enough that the ISR is asserted when checking
• no second increment should be performed.
  The nanosecond value is high and the second value must NOT be touched.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Tests

• Test configuration (please complete the following information):
  • Hardware setting: FRDM_K64F
  • Toolchain: GCC 12.2, zephyr SDK 17.0
  • Test Tool preparation:
  • Any other dependencies:
• Test executed
  Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
  • Build Test
  • Run Test

[zejiang0jason]

add @stanislav-poboril to review this change, and check whether OK to integrate in SDK.

Hi @theadib , the MCUX SDK driver development has been moved to: https://github.com/nxp-mcuxpresso/mcuxsdk-core, please use the new repo, thanks.

[stanislav-poboril]

Hi @theadib,

I am not sure about this change. Some applications might need to perform different corrections as well.
Consider this code from the SDK enet_txrx_ptp1588_transfer example:

            result = ENET_ReadFrame(EXAMPLE_ENET, &g_handle, data, length, 0, &ts);
            if (result == kStatus_Success)
            {
                ENET_Ptp1588GetTimer(EXAMPLE_ENET, &g_handle, &rxPtpTime);
                /* If latest timestamp reloads after getting from Rx BD, then second - 1 to make sure the actual Rx
                 * timestamp is accurate */
                if (rxPtpTime.nanosecond < ts)
                {
                    rxPtpTime.second--;
                }

                PRINTF(" A frame received. the length %d ", length);
                PRINTF(" the timestamp is %d second,", (uint32_t)rxPtpTime.second);
                PRINTF(" %d nanosecond  \r\n", ts);

It reads nanosecond part from the descriptor of the received frame.
It also calls ENET_Ptp1588GetTimer to get the seconds information only.
If the ENET_Ptp1588GetTimer nanosecond is smaller than ts, it adjusts for the wrap which occurred between reading a frame and calling ENET_Ptp1588GetTimer.

So this is a different kind of wrap, but the question is how many corner cases we could still get and if the probability of having wrong seconds part of the timestamp would be higher than the probability of reading high nanoseconds part in ENET_Ptp1588GetTimer before the wrap but still increasing the seconds.

[theadib]

hello @stanislav-poboril thanks for your feedback,
I noticed the odd behaviour when I was measuring the PPS pulse from GNSS receiver. That comes sharp at full second.
It happened that the reported second value did not really match to the time that should have past.

I hope you agree the patch will solve that situation.

To me it is important the call to ENET_Ptp1588GetTimer() has a nanosecond AND second value matching the same point in time.
My workaround was to read the time twice and then to correct. But it is better that the HAL does the things correct in the first place.
And if the HAL function ENET_Ptp1588GetTimer() is correct then the correction in the RX function is still valid and needed as well.
It also assumes that the nanosecond value and the second value are corresponding to each other.

Regards, Adib.

[stanislav-poboril]

Hello @theadib,

Thank you for the additional details. I am ok with the change.
It should just go to a newer repository as @zejiang0jason mentioned.

Thanks,
Stanislav