On 15.02.22 14:34, Patrice CHOTARD wrote: > Hi Jan > > On 2/15/22 14:00, Jan Kiszka wrote: >> On 15.02.22 12:56, Patrice CHOTARD wrote: >>> Hi Jan >>> >>> On 2/14/22 16:21, Jan Kiszka wrote: >>>> On 04.01.22 08:42, Patrice Chotard wrote: >>>>> When OF_LIVE flag is enabled on a 64 bits platform, there is an >>>>> issue when dev_read_addr() is called and need to perform an address >>>>> translation using __of_translate_address(). >>>>> >>>>> In case of error, __of_translate_address() return's value is OF_BAD_ADDR >>>>> (wich is defined in include/dm/of.h to ((u64)-1) = 0xffffffffffffffff). >>>>> The return value of dev_read_addr() is often compared to FDT_ADDR_T_NONE >>>>> which is defined as (-1U) = 0xffffffff. >>>>> In this case the comparison is always false. >>>>> >>>>> To fix this issue, define FDT_ADDR_T_NONE to (ulong)(-1) in case of >>>>> AARCH64. Update accordingly related tests. >>>>> >>>>> Signed-off-by: Patrice Chotard <patrice.chot...@foss.st.com> >>>>> >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - define FDT_ADDR_T_NONE as ((ulong)(-1)) and keep OF_BAD_ADDR unchanged >>>>> >>>>> include/fdtdec.h | 5 ++++- >>>>> test/dm/ofnode.c | 2 +- >>>>> test/dm/pci.c | 4 ++-- >>>>> test/dm/test-fdt.c | 2 +- >>>>> 4 files changed, 8 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/include/fdtdec.h b/include/fdtdec.h >>>>> index 6c7ab887b2..e9e2916d6e 100644 >>>>> --- a/include/fdtdec.h >>>>> +++ b/include/fdtdec.h >>>>> @@ -24,16 +24,19 @@ >>>>> typedef phys_addr_t fdt_addr_t; >>>>> typedef phys_size_t fdt_size_t; >>>>> >>>>> -#define FDT_ADDR_T_NONE (-1U) >>>>> #define FDT_SIZE_T_NONE (-1U) >>>>> >>>>> #ifdef CONFIG_PHYS_64BIT >>>>> +#define FDT_ADDR_T_NONE ((ulong)(-1)) >>>>> + >>>>> #define fdt_addr_to_cpu(reg) be64_to_cpu(reg) >>>>> #define fdt_size_to_cpu(reg) be64_to_cpu(reg) >>>>> #define cpu_to_fdt_addr(reg) cpu_to_be64(reg) >>>>> #define cpu_to_fdt_size(reg) cpu_to_be64(reg) >>>>> typedef fdt64_t fdt_val_t; >>>>> #else >>>>> +#define FDT_ADDR_T_NONE (-1U) >>>>> + >>>>> #define fdt_addr_to_cpu(reg) be32_to_cpu(reg) >>>>> #define fdt_size_to_cpu(reg) be32_to_cpu(reg) >>>>> #define cpu_to_fdt_addr(reg) cpu_to_be32(reg) >>>>> diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c >>>>> index cea0746bb3..e6c925eba6 100644 >>>>> --- a/test/dm/ofnode.c >>>>> +++ b/test/dm/ofnode.c >>>>> @@ -286,7 +286,7 @@ static int dm_test_ofnode_get_reg(struct >>>>> unit_test_state *uts) >>>>> ut_assert(ofnode_valid(node)); >>>>> addr = ofnode_get_addr(node); >>>>> size = ofnode_get_size(node); >>>>> - ut_asserteq(FDT_ADDR_T_NONE, addr); >>>>> + ut_asserteq_64(FDT_ADDR_T_NONE, addr); >>>>> ut_asserteq(FDT_SIZE_T_NONE, size); >>>>> >>>>> node = ofnode_path("/translation-test@8000/noxlatebus@3,300/dev@42"); >>>>> diff --git a/test/dm/pci.c b/test/dm/pci.c >>>>> index fa2e4a8559..00e4440a9d 100644 >>>>> --- a/test/dm/pci.c >>>>> +++ b/test/dm/pci.c >>>>> @@ -331,10 +331,10 @@ static int dm_test_pci_addr_live(struct >>>>> unit_test_state *uts) >>>>> struct udevice *swap1f, *swap1; >>>>> >>>>> ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &swap1f)); >>>>> - ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1f)); >>>>> + ut_asserteq_64(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1f)); >>>>> >>>>> ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1, 0), &swap1)); >>>>> - ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1)); >>>>> + ut_asserteq_64(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1)); >>>>> >>>>> return 0; >>>>> } >>>>> diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c >>>>> index 8866d4d959..e1de066226 100644 >>>>> --- a/test/dm/test-fdt.c >>>>> +++ b/test/dm/test-fdt.c >>>>> @@ -768,7 +768,7 @@ static int dm_test_fdt_livetree_writing(struct >>>>> unit_test_state *uts) >>>>> /* Test setting generic properties */ >>>>> >>>>> /* Non-existent in DTB */ >>>>> - ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr(dev)); >>>>> + ut_asserteq_64(FDT_ADDR_T_NONE, dev_read_addr(dev)); >>>>> /* reg = 0x42, size = 0x100 */ >>>>> ut_assertok(ofnode_write_prop(node, "reg", 8, >>>>> "\x00\x00\x00\x42\x00\x00\x01\x00")); >>>> >>>> Since this commit, I'm getting this dev_get_priv warning: >>>> >>>> [...] >>>> U-Boot 2022.01-00766-g9876ae7db6d-dirty (Feb 14 2022 - 16:15:21 +0100) >>>> >>>> Model: SIMATIC IOT2050 Basic >>>> DRAM: 1 GiB >>>> Core: 94 devices, 28 uclasses, devicetree: separate >>>> WDT: Not starting watchdog@40610000 >>>> MMC: mmc@4fa0000: 0 >>>> Loading Environment from SPIFlash... SF: Detected w25q128 with page size >>>> 256 Bytes, erase size 64 KiB, total 16 MiB >>>> drivers/dma/ti/k3-udma.c:1744 __dev_get_priv: null device >>>> drivers/dma/ti/k3-udma.c:1744 __dev_get_priv: null device >>>> OK >>>> In: serial >>>> Out: serial >>>> Err: serial >>>> Net: No ethernet found. >>>> Hit any key to stop autoboot: 0 >>>> >>>> (I've instrumented dev_get_priv to tell me file:line) >>>> >>>> Is that a sleeping problem surfaced by the commit or a real regression? >>>> I can boot, though. >>>> >>>> Jan >>>> >>> >>> It should be interesting to understand why uclass_get_device_by_phandle() >>> return tmp = NULL. >> >> Yep. >> >>> What's the return value of uclass_get_device_by_phandle() ? >>> >> >> -22, EINVAL. > > As EINVAL is one of the more "common" error value, i think you have to go > deeper > to see where the uclass_get_device_by_phandle() is failing. >
Sigh, I was hoping to get around debugging this myself. In any case: With this patch revert, the related code path is still taken, just successfully: ti-udma dma-controller@285c0000: ret=0 tmp=00000000bdf10750 Jan -- Siemens AG, Technology Competence Center Embedded Linux