| Message ID | 20220104074248.25015-1-patrice.chotard@foss.st.com |
|---|---|
| State | Accepted |
| Commit | 9876ae7db6da1cf8e106fcb46a36172fbb5781e5 |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [v2] dm: Fix OF_BAD_ADDR definition | expand |
On Tue, 4 Jan 2022 at 00:42, Patrice Chotard <patrice.chotard@foss.st.com> 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.chotard@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(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On Tue, 4 Jan 2022 at 00:42, Patrice Chotard <patrice.chotard@foss.st.com> 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.chotard@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(-) Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot-dm, thanks!
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.chotard@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
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.chotard@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. What's the return value of uclass_get_device_by_phandle() ? Patrice
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.chotard@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. Jan
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.chotard@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. Patrice > > Jan >
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.chotard@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
On 15.02.22 14:49, Jan Kiszka wrote: > 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.chotard@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 > To conclude this thread: The patch does what it is supposed to do, i.e. define the right FDT_ADDR_T_NONE so that comparisons finally work correctly on 64-bit archs. As they work correctly now, in this case in dev_remap_addr_name, they reveal that k3_nav_ringacc_init() tries to get a non-existent configuration register "cfg". So far it got -1LL as result, != NULL, and likely used that happily. The missing register came from a missing u-boot specific fragment in our board dts, compare to the TI reference board. Working on a fix. Jan
Hi Jan On 2/15/22 21:36, Jan Kiszka wrote: > On 15.02.22 14:49, Jan Kiszka wrote: >> 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.chotard@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 >> > > To conclude this thread: The patch does what it is supposed to do, i.e. > define the right FDT_ADDR_T_NONE so that comparisons finally work > correctly on 64-bit archs. > > As they work correctly now, in this case in dev_remap_addr_name, they > reveal that k3_nav_ringacc_init() tries to get a non-existent > configuration register "cfg". So far it got -1LL as result, != NULL, and > likely used that happily. The missing register came from a missing > u-boot specific fragment in our board dts, compare to the TI reference > board. Working on a fix. > > Jan > Thanks for the feedback ;-) Patrice
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"));
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.chotard@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(-)