Hi, I am doing some checking but it will be available soon.
M On 17. 03. 20 16:38, wqyoung wrote: > 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 (#8514): https://lists.yoctoproject.org/g/linux-yocto/message/8514 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]] -=-=-=-=-=-=-=-=-=-=-=-
