Hi Michal,Is the xilinx kernel 5.4 finished? I check the git tree https://github.com/Xilinx/linux-xlnx.git and there is no xlnx_rebase_v5.4 branch.
Thanks, Quanyang On 2020/3/17 下午11:25, Michal Simek wrote:
Hi, tbh I think that it will be good to start to think how to switch 5.4 to be based on xilinx rebase kernel and this code will be changed anyway. Thanks, Michal On 17. 03. 20 15:56, Quanyang Wang wrote:Hi Michal, It's strange that this issue only exists with SDK 4.19 patches for sdhci-of-arasan.c and gcc 9.2. Once the code is changed, this issue disappears. It means that the over optimization of gcc 9.2 is not the root cause. Since this issue breaks down the mmc controller in linux-yocto 5.4 kernel, I want to apply it as a workaround. I will send a V2 patch to modify description and fix some typos. Thanks, Quanyang On 3/17/20 5:24 PM, Michal Simek wrote:Hi, I have tested the latest Xilinx kernel based on v5.4 at github.com/Xilinx/linux-xlnx and I built it by toolchain you pointed me to and I can't see any issue. Driver has changed a little bit based on mainline discussion but handling these values is the same. $ dmesg | grep gcc [ 0.000000] Linux version 5.4.0-09934-ge977da2d1aee-dirty (monstr@monstr-desktop3) (gcc version 9.2.1 20191025 (GNU Toolchain for the A-profile Architecture 9.2-2019.12 (arm-9.10))) #82 SMP Tue Mar 17 10:17:09 CET 2020 $ dmesg | grep arasan [ 15.416035] arasan_dt_parse_clk_phases:0-63-63-0-63 0-0-183-54-0 0 [ 15.422308] arasan_dt_parse_clk_phases:0-72-60-0-60 72-135-48-72-135 0 As I said I have no problem with changing code to look as you have below but I can't see any issue with gcc 9.2 in connection to this. Manish: Can you please take a look at patch below and create a mainline patch to follow this style if you consider this better that what you use now. Thanks, Michal On 10. 03. 20 15:41, Quanyang Wang wrote: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 (#8513): https://lists.yoctoproject.org/g/linux-yocto/message/8513 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]] -=-=-=-=-=-=-=-=-=-=-=-
