Skip to content
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

modules: hal: nxp: update to include fix for CACHE64_GetInstanceByAddr() #81103

Conversation

danieldegrasse
Copy link
Collaborator

CACHE64_GetInstanceByAddr() function was asserting when an instance was requested for an invalid address that the CACHE64 controller does not manage. This behavior is not correct, as the CACHE64 management functions check to see if the instance number returned by this function is out of range (and if so, simply return without modifying the cache).

This assertion was causing a failure within the USDHC driver, which performs a cache clean/invalidate for tx/rx transfers within the HAL layer. When a transfer was run using a data buffer not in the CACHE64 address range, this assertion failed and caused the application to crash

Fixes #80901

@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Nov 7, 2024
@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 7, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@6e7d5cf zephyrproject-rtos/hal_nxp@c410b73 (master) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Nov 7, 2024
@dleach02 dleach02 added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Nov 7, 2024
@dleach02 dleach02 self-assigned this Nov 7, 2024
dleach02
dleach02 previously approved these changes Nov 7, 2024
@dleach02 dleach02 added this to the v4.0.0 milestone Nov 7, 2024
@mmahadevan108
Copy link
Collaborator

@danieldegrasse , please update the HAL SHA

@mmahadevan108 mmahadevan108 added the bug The issue is a bug, or the PR is fixing a bug label Nov 7, 2024
CACHE64_GetInstanceByAddr() function was asserting when an instance was
requested for an invalid address that the CACHE64 controller does not
manage. This behavior is not correct, as the CACHE64 management
functions check to see if the instance number returned by this function
is out of range (and if so, simply return without modifying the cache).

This assertion was causing a failure within the USDHC driver, which
performs a cache clean/invalidate for tx/rx transfers within the HAL
layer. When a transfer was run using a data buffer not in the CACHE64
address range, this assertion failed and caused the application to crash

Fixes zephyrproject-rtos#80901

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse
Copy link
Collaborator Author

@danieldegrasse , please update the HAL SHA

Done, thanks for the ping

@danieldegrasse danieldegrasse force-pushed the fix/rt685-usdhc-cache-assert branch from bf75e3b to 109185b Compare November 7, 2024 23:07
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Nov 7, 2024
@mmahadevan108 mmahadevan108 self-assigned this Nov 8, 2024
@hakehuang
Copy link
Collaborator

@danieldegrasse this pr fix part of this sdmmc issue, still another one

b8466e0c951b8a159ed0a0465ed831497668197b is the first bad commit
commit b8466e0c951b8a159ed0a0465ed831497668197b
Author: Vit Stanicek <[email protected]>
Date:   Mon Aug 5 10:40:04 2024 +0200

    boards: mimxrt685_evk/mimxrt685s/cm33: Enable DMIC

    Enable DMIC clock in soc.c - attach to chip's audio PLL. Add pinmux
    definitions for the DMIC peripheral. Add nodes to SoC's device tree for
    the DMIC peripheral and its audio channels. Configure the DMIC
    peripheral in board's device tree to enable audio capture.

    Signed-off-by: Vit Stanicek <[email protected]>

 .../nxp/mimxrt685_evk/mimxrt685_evk-pinctrl.dtsi   | 10 ++++
 .../mimxrt685_evk_mimxrt685s_cm33.dts              | 27 ++++++++-
 dts/arm/nxp/nxp_rt6xx_common.dtsi                  | 66 ++++++++++++++++++++++
 soc/nxp/imxrt/imxrt6xx/cm33/soc.c                  | 12 ++++
 4 files changed, 114 insertions(+), 1 deletion(-)

@danieldegrasse
Copy link
Collaborator Author

@danieldegrasse this pr fix part of this sdmmc issue, still another one


b8466e0c951b8a159ed0a0465ed831497668197b is the first bad commit

commit b8466e0c951b8a159ed0a0465ed831497668197b

Author: Vit Stanicek <[email protected]>

Date:   Mon Aug 5 10:40:04 2024 +0200



    boards: mimxrt685_evk/mimxrt685s/cm33: Enable DMIC



    Enable DMIC clock in soc.c - attach to chip's audio PLL. Add pinmux

    definitions for the DMIC peripheral. Add nodes to SoC's device tree for

    the DMIC peripheral and its audio channels. Configure the DMIC

    peripheral in board's device tree to enable audio capture.



    Signed-off-by: Vit Stanicek <[email protected]>



 .../nxp/mimxrt685_evk/mimxrt685_evk-pinctrl.dtsi   | 10 ++++

 .../mimxrt685_evk_mimxrt685s_cm33.dts              | 27 ++++++++-

 dts/arm/nxp/nxp_rt6xx_common.dtsi                  | 66 ++++++++++++++++++++++

 soc/nxp/imxrt/imxrt6xx/cm33/soc.c                  | 12 ++++

 4 files changed, 114 insertions(+), 1 deletion(-)

Locally the sdmmc test passes for me with this fix. Do you still see a failure on your board?

@hakehuang
Copy link
Collaborator

hakehuang commented Nov 8, 2024

Locally the sdmmc test passes for me with this fix. Do you still see a failure on your board?

sorry my bad. this PR fixes all issues.

*** Booting Zephyr OS build v4.0.0-rc2-129-g109185bb15dd ***
Running TESTSUITE sd_stack
===================================================================
START - test_0_init
 PASS - test_0_init in 1.142 seconds
===================================================================
START - test_card_config
Card voltage: 3.3V
Card timing: High Speed
Bus Frequency: 50000000 Hz
Card type: SDMMC
Card spec: 3.0
 PASS - test_card_config in 0.010 seconds
===================================================================
START - test_ioctl
SD card reports sector count of 7744512
SD card reports sector size of 512
 PASS - test_ioctl in 0.007 seconds
===================================================================
START - test_read
 PASS - test_read in 0.056 seconds
===================================================================
START - test_rw
 PASS - test_rw in 0.827 seconds
===================================================================
START - test_write
 PASS - test_write in 0.134 seconds
===================================================================
TESTSUITE sd_stack succeeded

------ TESTSUITE SUMMARY START ------

SUITE PASS - 100.00% [sd_stack]: pass = 6, fail = 0, skip = 0, total = 6 duration = 2.176 seconds
 - PASS - [sd_stack.test_0_init] duration = 1.142 seconds
 - PASS - [sd_stack.test_card_config] duration = 0.010 seconds
 - PASS - [sd_stack.test_ioctl] duration = 0.007 seconds
 - PASS - [sd_stack.test_read] duration = 0.056 seconds
 - PASS - [sd_stack.test_rw] duration = 0.827 seconds
 - PASS - [sd_stack.test_write] duration = 0.134 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
RunID: e6aefb7b33609a927a654a6bcc536d46
PROJECT EXECUTION SUCCESSFUL

@mmahadevan108 mmahadevan108 merged commit 2ad01d9 into zephyrproject-rtos:main Nov 8, 2024
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug manifest manifest-hal_nxp size: XS A PR changing only a single line of code Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: subsys: sd : sdmmc: [mimxrt685_evk]: sd card conflicts with default dmic enablement
5 participants