Hi Michal,
As below is the detail steps to reproduce this issue in SDK kernel:
1). the aarch-none-linux-gnu-gcc v9.2 is here
https://developer.arm.com/-/media/Files/downloads/gnu-a/9.2-2019.12/binrel/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz?revision=61c3be5d-5175-4db6-9030-b565aae9f766&la=en&hash=0A37024B42028A9616F56A51C2D20755C5EBBCD
I use yocto 9.2.0 gcc can also reproduce this issue.
2). build xilinx 4.19 kernel with this gcc and boot *zcu102* board.
The arasan_zynqmp_set_tap_delayj will use random values to initialize
the register of mmc controller.
And when using the printk to print the values in itapdly, you will found
that the values are not ZYNQMP_ITAP_DELAYS but random values.
This is the debug patch to print the value:
$ git diff
diff --git a/drivers/mmc/host/sdhci-of-arasan.c
b/drivers/mmc/host/sdhci-of-arasan.c
index 2d38a4325fff..3c366456f703 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -971,6 +971,7 @@ static void arasan_dt_parse_tap_delays(struct device
*dev)
otapdly = (u32 [MMC_TIMING_MMC_HS400 + 1])
VERSAL_OTAP_DELAYS;
}
+ printk("-%x-%x-%x\r\n", itapdly[0], itapdly[1],itapdly[2]);
arasan_dt_read_tap_delay(dev, itapdly, MMC_TIMING_SD_HS,
"xlnx,itap-delay-sd-hsd");
arasan_dt_read_tap_delay(dev, itapdly, MMC_TIMING_UHS_SDR25,
This is the print result before applying this patch:
[ 5.505387] cdns-wdt fd4d0000.watchdog: Xilinx Watchdog Timer with
timeout 60s
*[ 5.514214] -2f0cdf00-9b4d5b9a-935ba90*
[ 5.549141] mmc0: SDHCI controller on ff170000.mmc [ff170000.mmc]
using ADMA 64-bit
[ 5.565127] input: gpio-keys as /devices/platform/gpio-keys/input/input0
wg
This is the print value after applying the patch
[ 5.484797] cdns-wdt fd4d0000.watchdog: Xilinx Watchdog Timer with
timeout 60s
*[ 5.493622] -0-15-15*
[ 5.525589] mmc0: SDHCI controller on ff170000.mmc [ff170000.mmc]
using ADMA 64-bit
[ 5.541632] input: gpio-keys as /devices/platform/gpio-keys/input/input0
I don't know if this is a bug of GCC v9.2.0. But it seems that using
the array of integers is a more reasonable method.
Thanks,
Quanyang
On 3/10/20 8:17 PM, Michal Simek wrote:
+ Manish,
On 10. 03. 20 12:33, [email protected] wrote:
From: Quanyang Wang <[email protected]>
When assigning value to a pointer as below:
itapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) {0x0, 0x15, 0x15...0x0};
then using gcc 9.2.0 with -O2 to compile, the pointer itapdly will point to
the stack of the function, and the values in this area is not initialized.
The disassemble code is as below:
ffffffc010a389b8: 940089ac bl ffffffc010a5b068
<of_device_is_compatible>
ffffffc010a389bc: 34001100 cbz w0, ffffffc010a38bdc
<arasan_dt_parse_tap_delays+0x264>
itapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) ZYNQMP_ITAP_DELAYS;
otapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) ZYNQMP_OTAP_DELAYS;
ffffffc010a389c0: 9101d3f4 add x20, sp, #0x74
itapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) ZYNQMP_ITAP_DELAYS;
ffffffc010a389c4: 910123f5 add x21, sp, #0x48
int ret = of_property_read_variable_u32_array(np, propname,
out_values,
This results that the pointer itapdly is not the address of the array
ZYNQMP_ITAP_DELAYS but a stack address which contains random values.
So use an array of integers to avoid the over optimization of the new
compiler.
Signed-off-by: Quanyang Wang <[email protected]>
--
drivers/mmc/host/sdhci-of-arasan.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
---
drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
This look bit weird.
I will fix it in V2.
diff --git a/drivers/mmc/host/sdhci-of-arasan.c
b/drivers/mmc/host/sdhci-of-arasan.c
index bc9ca6ae20ce..34dcded79b78 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -52,15 +52,15 @@
#define SDHCI_ITAPDLY_ENABLE 0x100
#define SDHCI_OTAPDLY_ENABLE 0x40
-#define ZYNQMP_ITAP_DELAYS {0x0, 0x15, 0x15, 0x0, 0x15, 0x0,\
- 0x0, 0x3D, 0x12, 0x0, 0x0}
-#define ZYNQMP_OTAP_DELAYS {0x0, 0x5, 0x6, 0x0, 0x5, 0x3,\
- 0x3, 0x4, 0x6, 0x3, 0x0}
+static u32 zynqmp_itap_delays[MMC_TIMING_MMC_HS400+1] = {0x0, 0x15, 0x15, 0x0,
missing spaces around +
Will fix it in V2.
+ 0x15, 0x0, 0x0, 0x3D, 0x12, 0x0, 0x0};
+static u32 zynqmp_otap_delays[MMC_TIMING_MMC_HS400+1] = {0x0, 0x5, 0x6, 0x0,
ditto.
Will fix it in V2.
+ 0x5, 0x3, 0x3, 0x4, 0x6, 0x3, 0x0};
-#define VERSAL_ITAP_DELAYS {0x0, 0x2C, 0x2C, 0x0, 0x2C, 0x0,\
- 0x0, 0x36, 0x1E, 0x0, 0x0}
-#define VERSAL_OTAP_DELAYS {0x0, 0x5, 0x4, 0x0, 0x4, 0x3,\
- 0x2, 0x3, 0x5, 0x2, 0x0}
+static u32 versal_itap_delays[MMC_TIMING_MMC_HS400 + 1] = {0x0, 0x2C, 0x2C,
+ 0x0, 0x2C, 0x0, 0x0, 0x36, 0x1E, 0x0, 0x0};
+static u32 versal_otap_delays[MMC_TIMING_MMC_HS400 + 1] = {0x0, 0x5, 0x4,
+ 0x0, 0x4, 0x3, 0x2, 0x3, 0x5, 0x2, 0x0};
#define MMC_BANK2 0x2
@@ -995,15 +995,15 @@ static void arasan_dt_parse_tap_delays(struct device *dev)
int i;
if (of_device_is_compatible(pdev->dev.of_node, "xlnx,zynqmp-8.9a")) {
- itapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) ZYNQMP_ITAP_DELAYS;
- otapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) ZYNQMP_OTAP_DELAYS;
+ itapdly = zynqmp_itap_delays;
+ otapdly = zynqmp_otap_delays;
if (sdhci_arasan->mio_bank == MMC_BANK2) {
otapdly[MMC_TIMING_UHS_SDR104] = 0x2;
otapdly[MMC_TIMING_MMC_HS200] = 0x2;
}
} else {
- itapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) VERSAL_ITAP_DELAYS;
- otapdly = (u32 [MMC_TIMING_MMC_HS400 + 1]) VERSAL_OTAP_DELAYS;
+ itapdly = versal_itap_delays;
+ otapdly = versal_otap_delays;
}
arasan_dt_read_tap_delay(dev, itapdly, MMC_TIMING_SD_HS,
Manish: Can you please take a look?
I can't see any issue with this.
Thanks,
Michal
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#8455):
https://lists.yoctoproject.org/g/linux-yocto/message/8455
Mute This Topic: https://lists.yoctoproject.org/mt/71854797/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
