* [PATCH v2] dm: Fix OF_BAD_ADDR definition
@ 2022-01-04 7:42 Patrice Chotard
2022-01-21 15:20 ` Simon Glass
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Patrice Chotard @ 2022-01-04 7:42 UTC (permalink / raw)
To: u-boot; +Cc: Patrice CHOTARD, Patrick DELAUNAY, U-Boot STM32, Simon Glass
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"));
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-01-04 7:42 [PATCH v2] dm: Fix OF_BAD_ADDR definition Patrice Chotard
@ 2022-01-21 15:20 ` Simon Glass
2022-01-26 15:37 ` Simon Glass
2022-02-14 15:21 ` Jan Kiszka
2 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2022-01-21 15:20 UTC (permalink / raw)
To: Patrice Chotard; +Cc: U-Boot Mailing List, Patrick DELAUNAY, U-Boot STM32
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-01-04 7:42 [PATCH v2] dm: Fix OF_BAD_ADDR definition Patrice Chotard
2022-01-21 15:20 ` Simon Glass
@ 2022-01-26 15:37 ` Simon Glass
2022-02-14 15:21 ` Jan Kiszka
2 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2022-01-26 15:37 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Patrice CHOTARD, Patrick DELAUNAY, U-Boot STM32
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!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-01-04 7:42 [PATCH v2] dm: Fix OF_BAD_ADDR definition Patrice Chotard
2022-01-21 15:20 ` Simon Glass
2022-01-26 15:37 ` Simon Glass
@ 2022-02-14 15:21 ` Jan Kiszka
2022-02-15 11:56 ` Patrice CHOTARD
2 siblings, 1 reply; 10+ messages in thread
From: Jan Kiszka @ 2022-02-14 15:21 UTC (permalink / raw)
To: Patrice Chotard, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-02-14 15:21 ` Jan Kiszka
@ 2022-02-15 11:56 ` Patrice CHOTARD
2022-02-15 13:00 ` Jan Kiszka
0 siblings, 1 reply; 10+ messages in thread
From: Patrice CHOTARD @ 2022-02-15 11:56 UTC (permalink / raw)
To: Jan Kiszka, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-02-15 11:56 ` Patrice CHOTARD
@ 2022-02-15 13:00 ` Jan Kiszka
2022-02-15 13:34 ` Patrice CHOTARD
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kiszka @ 2022-02-15 13:00 UTC (permalink / raw)
To: Patrice CHOTARD, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-02-15 13:00 ` Jan Kiszka
@ 2022-02-15 13:34 ` Patrice CHOTARD
2022-02-15 13:49 ` Jan Kiszka
0 siblings, 1 reply; 10+ messages in thread
From: Patrice CHOTARD @ 2022-02-15 13:34 UTC (permalink / raw)
To: Jan Kiszka, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-02-15 13:34 ` Patrice CHOTARD
@ 2022-02-15 13:49 ` Jan Kiszka
2022-02-15 20:36 ` Jan Kiszka
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kiszka @ 2022-02-15 13:49 UTC (permalink / raw)
To: Patrice CHOTARD, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-02-15 13:49 ` Jan Kiszka
@ 2022-02-15 20:36 ` Jan Kiszka
2022-02-16 8:24 ` Patrice CHOTARD
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kiszka @ 2022-02-15 20:36 UTC (permalink / raw)
To: Patrice CHOTARD, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dm: Fix OF_BAD_ADDR definition
2022-02-15 20:36 ` Jan Kiszka
@ 2022-02-16 8:24 ` Patrice CHOTARD
0 siblings, 0 replies; 10+ messages in thread
From: Patrice CHOTARD @ 2022-02-16 8:24 UTC (permalink / raw)
To: Jan Kiszka, u-boot, Simon Glass, Nishanth Menon
Cc: Patrick DELAUNAY, U-Boot STM32
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-02-16 8:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 7:42 [PATCH v2] dm: Fix OF_BAD_ADDR definition Patrice Chotard
2022-01-21 15:20 ` Simon Glass
2022-01-26 15:37 ` Simon Glass
2022-02-14 15:21 ` Jan Kiszka
2022-02-15 11:56 ` Patrice CHOTARD
2022-02-15 13:00 ` Jan Kiszka
2022-02-15 13:34 ` Patrice CHOTARD
2022-02-15 13:49 ` Jan Kiszka
2022-02-15 20:36 ` Jan Kiszka
2022-02-16 8:24 ` Patrice CHOTARD
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.