New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add memory barriers to STM32F7xx Ethernet #5720

Merged
merged 1 commit into from Dec 22, 2017

Conversation

Projects
None yet
6 participants
@kjbracey-arm
Contributor

kjbracey-arm commented Dec 18, 2017

Pending official update from STM, add memory barriers to the Ethernet
HAL code for the STM32F7xx family.

Cortex-M7 has a merging write buffer that is not automatically flushed
by accesses to devices, so without these DMBs, we sometimes lose synch
with the transmitter.

The DMBs are architecturally needed in every version of this HAL, but
adding just to the STM32F7 version for now to clear test, as the
problem has only been observed on Cortex-M7-based devices.

Fixes #5622.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 18, 2017

@ARMmbed/team-st-mcd please review

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 18, 2017

/morph build

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Dec 18, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Dec 18, 2017

Build : SUCCESS

Build number : 713
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5720/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 18, 2017

/morph export-build

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 20, 2017

This will be ready once travis fix is landed, will need to be rebased to get the travis fix propagated.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 20, 2017

@kjbracey-arm Can you rebase (travis fix to get in to resolve the failure), I'll restart Ci asap

Add memory barriers to STM32F7xx Ethernet
Pending official update from STM, add memory barriers to the Ethernet
HAL code for the STM32F7xx family.

Cortex-M7 has a merging write buffer that is not automatically flushed
by accesses to devices, so without these DMBs, we sometimes lose synch
with the transmitter.

The DMBs are architecturally needed in every version of this HAL, but
adding just to the STM32F7 version for now to clear test, as the
problem has only been observed on Cortex-M7-based devices.

Fixes #5622.

@kjbracey-arm kjbracey-arm force-pushed the kjbracey-arm:STM32F7_DMB branch from b086316 to 05e2ae7 Dec 20, 2017

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Dec 20, 2017

Rebased.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 20, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Dec 20, 2017

Build : SUCCESS

Build number : 727
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5720/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 20, 2017

@kjbracey-arm The latest test result reports one failure for k64f ARM: [1513779874.54][CONN][RXD] Thread 00000000 error -4: Parameter error - isthis known issue, I recall seeing your findings from this morning. How can we fix this?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 20, 2017

Need to have few rounds to see if it fixes the issue

/morph test

@mbed-ci

This comment has been minimized.

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Dec 21, 2017

Not sure if multiple CI successes on this PR should raise confidence - the observation was that the success rate was either near 0% or near 100% for a specific image, and the theory was that depends on alignment of the transmit buffers in the image. I'd expect you to get the same result on every test, whether or not this was fixing it.

If I could see that the image being tested had the transmit buffers at a +0x1c offset, or if current tip of master was failing, I'd be semi-convinced.

On the K64F thing starting to be visible (#5680) - have we just started testing debug images? Or has the network segment they're on just gotten busier? I'll figure out a patch for it.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Dec 22, 2017

@0xc0170 0xc0170 merged commit dd5bd73 into ARMmbed:master Dec 22, 2017

17 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2-ATMEL Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-SILICON_LABS Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM Local mbed2-STM testing has passed
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Dec 28, 2017

@kjbracey-arm kjbracey-arm deleted the kjbracey-arm:STM32F7_DMB branch Jan 2, 2018

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jan 3, 2018

Hi
Issue still occur even with that patch ? :-(
#5570 (comment)

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 3, 2018

Issue still occur even with that patch ? :-(

haven't seen it on master. I restarted also build phase in the referenced PR, will wait for results

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Jan 3, 2018

Would that need to be rebased? Does the build test PR tip or merge result?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 3, 2018

Would that need to be rebased? Does the build test PR tip or merge result?

CI merges with master, but we triggered only test phase (build was old one as I noticed), so I restarted the build as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment