[PATCH v2] rockchip: timer: add OF_PLATDATA support for dw-apb-timer
The Rockchip rk3066 SoC has 3 dw-apb-timer nodes. U-boot is compiled with OF_PLATDATA TPL/SPL options, so add OF_PLATDATA support for the dw-apb-timer. Also change driver name to be able to compile with U-boot scripts. No reset OF_PLATDATA support was added, because the rk3066 nodes don't need/have them.
Signed-off-by: Johan Jonker jbx6244@gmail.com ---
Changed V2: replace if (IS_ENABLED(OF_REAL)) by #if CONFIG_IS_ENABLED(OF_REAL) --- drivers/timer/dw-apb-timer.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 9aed5dd2..9ba8695f 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -8,6 +8,7 @@ #include <common.h> #include <dm.h> #include <clk.h> +#include <dt-structs.h> #include <malloc.h> #include <reset.h> #include <timer.h> @@ -25,6 +26,12 @@ struct dw_apb_timer_priv { struct reset_ctl_bulk resets; };
+struct dw_apb_timer_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_snps_dw_apb_timer dtplat; +#endif +}; + static u64 dw_apb_timer_get_count(struct udevice *dev) { struct dw_apb_timer_priv *priv = dev_get_priv(dev); @@ -43,7 +50,18 @@ static int dw_apb_timer_probe(struct udevice *dev) struct dw_apb_timer_priv *priv = dev_get_priv(dev); struct clk clk; int ret; +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dw_apb_timer_plat *plat = dev_get_plat(dev); + struct dtd_snps_dw_apb_timer *dtplat = &plat->dtplat;
+ priv->regs = dtplat->reg[0]; + + ret = clk_get_by_phandle(dev, &dtplat->clocks[0], &clk); + if (ret < 0) + return ret; + + uc_priv->clock_rate = dtplat->clock_frequency; +#else ret = reset_get_bulk(dev, &priv->resets); if (ret) dev_warn(dev, "Can't get reset: %d\n", ret); @@ -57,7 +75,7 @@ static int dw_apb_timer_probe(struct udevice *dev) uc_priv->clock_rate = clk_get_rate(&clk);
clk_free(&clk); - +#endif /* init timer */ writel(0xffffffff, priv->regs + DW_APB_LOAD_VAL); writel(0xffffffff, priv->regs + DW_APB_CURR_VAL); @@ -68,10 +86,11 @@ static int dw_apb_timer_probe(struct udevice *dev)
static int dw_apb_timer_of_to_plat(struct udevice *dev) { +#if CONFIG_IS_ENABLED(OF_REAL) struct dw_apb_timer_priv *priv = dev_get_priv(dev);
priv->regs = dev_read_addr(dev); - +#endif return 0; }
@@ -91,13 +110,14 @@ static const struct udevice_id dw_apb_timer_ids[] = { {} };
-U_BOOT_DRIVER(dw_apb_timer) = { - .name = "dw_apb_timer", +U_BOOT_DRIVER(snps_dw_apb_timer) = { + .name = "snps_dw_apb_timer", .id = UCLASS_TIMER, .ops = &dw_apb_timer_ops, .probe = dw_apb_timer_probe, .of_match = dw_apb_timer_ids, - .of_to_plat = dw_apb_timer_of_to_plat, + .of_to_plat = dw_apb_timer_of_to_plat, .remove = dw_apb_timer_remove, .priv_auto = sizeof(struct dw_apb_timer_priv), + .plat_auto = sizeof(struct dw_apb_timer_plat), };
Hi Johan,
On Tue, 4 Jan 2022 at 19:15, Johan Jonker jbx6244@gmail.com wrote:
The Rockchip rk3066 SoC has 3 dw-apb-timer nodes. U-boot is compiled with OF_PLATDATA TPL/SPL options, so add OF_PLATDATA support for the dw-apb-timer. Also change driver name to be able to compile with U-boot scripts. No reset OF_PLATDATA support was added, because the rk3066 nodes don't need/have them.
Signed-off-by: Johan Jonker jbx6244@gmail.com
Changed V2: replace if (IS_ENABLED(OF_REAL)) by #if CONFIG_IS_ENABLED(OF_REAL)
drivers/timer/dw-apb-timer.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
This seems OK but you have included unrelated changes (whitespace) which should go in a separate patch.
Can you use if() instead of #if for the CONFIG_IS_ENABLED(OF_REAL) ?
Regards, Simon
Hi Simon,
Thanks you for your comments. Shown below are the objdump results of the full U-boot binary dw_apb_timer_of_to_plat() function. Same goes for the dw_apb_timer_probe() function. With if (IS_ENABLED(OF_REAL)) I don't get a useful timer result (boot hangs after timer probe, because in full U-boot the reg value isn't correct defined. Part of the init probe is missing.
Could you try it yourself? Please advise what options we have.
Kind regards,
Johan Jonker
========================================================================== Patch version 1 with if (IS_ENABLED(OF_REAL)) {}:
arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-v1.asm
00000000 <dw_apb_timer_of_to_plat>: 0: 2000 movs r0, #0 2: 4770 bx lr
arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-spl-v1.asm arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-tpl-v1.asm ========================================================================== patch version 2 with #if CONFIG_IS_ENABLED(OF_REAL):
arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb- timer-20220105-v2.asm
00000000 <dw_apb_timer_of_to_plat>: 0: b538 push {r3, r4, r5, lr} 2: 4605 mov r5, r0 4: f7ff fffe bl 0 <dev_get_priv> 8: 4604 mov r4, r0 a: 4628 mov r0, r5 c: f7ff fffe bl 0 <devfdt_get_addr> 10: 6020 str r0, [r4, #0] 12: 2000 movs r0, #0 14: bd38 pop {r3, r4, r5, pc}
arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-spl-v2.asm arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-tpl-v2.asm
===========================================================================
On 1/5/22 3:03 PM, Simon Glass wrote:
Hi Johan,
On Tue, 4 Jan 2022 at 19:15, Johan Jonker jbx6244@gmail.com wrote:
The Rockchip rk3066 SoC has 3 dw-apb-timer nodes. U-boot is compiled with OF_PLATDATA TPL/SPL options, so add OF_PLATDATA support for the dw-apb-timer. Also change driver name to be able to compile with U-boot scripts. No reset OF_PLATDATA support was added, because the rk3066 nodes don't need/have them.
Signed-off-by: Johan Jonker jbx6244@gmail.com
Changed V2: replace if (IS_ENABLED(OF_REAL)) by #if CONFIG_IS_ENABLED(OF_REAL)
drivers/timer/dw-apb-timer.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
This seems OK but you have included unrelated changes (whitespace) which should go in a separate patch.
OK With whitespace did you mean this or something else?
- .of_to_plat = dw_apb_timer_of_to_plat, + .of_to_plat = dw_apb_timer_of_to_plat,
Can you use if() instead of #if for the CONFIG_IS_ENABLED(OF_REAL) ?
See comments above. Currently not it seems.
Regards, Simon
Hi,
In addition of the previous message, when I compile with the opposite of OF_REAL (= !OF_PLATDATA) it generates an error in SPL. Like to know why OF_REAL doesn't work. For these couple of extra lines to increase build coverage inside does it matter a lot by adding another messy set of #if .... Let me know if that's OK to leave it as it is for version 3 with extra white space patch.
Kind regards,
Johan Jonker
static int dw_apb_timer_of_to_plat(struct udevice *dev) { if (!IS_ENABLED(OF_PLATDATA)) { struct dw_apb_timer_priv *priv = dev_get_priv(dev);
priv->regs = dev_read_addr(dev); }
return 0; }
arm-linux-gnueabihf-ld.bfd: drivers/timer/dw-apb-timer.o: in function `dw_apb_timer_of_to_plat': /drivers/timer/dw-apb-timer.c:95: undefined reference to `dev_read_addr' arm-linux-gnueabihf-ld.bfd: drivers/timer/dw-apb-timer.o: in function `dw_apb_timer_probe': /drivers/timer/dw-apb-timer.c:73: undefined reference to `clk_get_by_index' make[1]: *** [scripts/Makefile.spl:509: spl/u-boot-spl] Error 1 make: *** [Makefile:2011: spl/u-boot-spl] Error 2
On 1/5/22 5:40 PM, Johan Jonker wrote:
Hi Simon,
Thanks you for your comments. Shown below are the objdump results of the full U-boot binary dw_apb_timer_of_to_plat() function. Same goes for the dw_apb_timer_probe() function. With if (IS_ENABLED(OF_REAL)) I don't get a useful timer result (boot hangs after timer probe, because in full U-boot the reg value isn't correct defined. Part of the init probe is missing.
Could you try it yourself? Please advise what options we have.
Kind regards,
Johan Jonker
========================================================================== Patch version 1 with if (IS_ENABLED(OF_REAL)) {}:
arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-v1.asm
00000000 <dw_apb_timer_of_to_plat>: 0: 2000 movs r0, #0 2: 4770 bx lr
arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-spl-v1.asm arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-tpl-v1.asm ========================================================================== patch version 2 with #if CONFIG_IS_ENABLED(OF_REAL):
arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb- timer-20220105-v2.asm
00000000 <dw_apb_timer_of_to_plat>: 0: b538 push {r3, r4, r5, lr} 2: 4605 mov r5, r0 4: f7ff fffe bl 0 <dev_get_priv> 8: 4604 mov r4, r0 a: 4628 mov r0, r5 c: f7ff fffe bl 0 <devfdt_get_addr> 10: 6020 str r0, [r4, #0] 12: 2000 movs r0, #0 14: bd38 pop {r3, r4, r5, pc}
arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-spl-v2.asm arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-tpl-v2.asm
===========================================================================
On 1/5/22 3:03 PM, Simon Glass wrote:
Hi Johan,
On Tue, 4 Jan 2022 at 19:15, Johan Jonker jbx6244@gmail.com wrote:
The Rockchip rk3066 SoC has 3 dw-apb-timer nodes. U-boot is compiled with OF_PLATDATA TPL/SPL options, so add OF_PLATDATA support for the dw-apb-timer. Also change driver name to be able to compile with U-boot scripts. No reset OF_PLATDATA support was added, because the rk3066 nodes don't need/have them.
Signed-off-by: Johan Jonker jbx6244@gmail.com
Changed V2: replace if (IS_ENABLED(OF_REAL)) by #if CONFIG_IS_ENABLED(OF_REAL)
drivers/timer/dw-apb-timer.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
This seems OK but you have included unrelated changes (whitespace) which should go in a separate patch.
OK With whitespace did you mean this or something else?
- .of_to_plat = dw_apb_timer_of_to_plat,
- .of_to_plat = dw_apb_timer_of_to_plat,
Can you use if() instead of #if for the CONFIG_IS_ENABLED(OF_REAL) ?
See comments above. Currently not it seems.
Regards, Simon
Hi Johan,
On Wed, 5 Jan 2022 at 09:40, Johan Jonker jbx6244@gmail.com wrote:
Hi Simon,
Thanks you for your comments. Shown below are the objdump results of the full U-boot binary dw_apb_timer_of_to_plat() function. Same goes for the dw_apb_timer_probe() function. With if (IS_ENABLED(OF_REAL)) I don't get a useful timer result (boot
You need CONFIG_IS_ENABLED(OF_REAL)
(please could you avoid top posting?)
Regards, Simon
hangs after timer probe, because in full U-boot the reg value isn't correct defined. Part of the init probe is missing.
Could you try it yourself? Please advise what options we have.
Kind regards,
Johan Jonker
========================================================================== Patch version 1 with if (IS_ENABLED(OF_REAL)) {}:
arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-v1.asm
00000000 <dw_apb_timer_of_to_plat>: 0: 2000 movs r0, #0 2: 4770 bx lr
arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-spl-v1.asm arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-tpl-v1.asm ========================================================================== patch version 2 with #if CONFIG_IS_ENABLED(OF_REAL):
arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb- timer-20220105-v2.asm
00000000 <dw_apb_timer_of_to_plat>: 0: b538 push {r3, r4, r5, lr} 2: 4605 mov r5, r0 4: f7ff fffe bl 0 <dev_get_priv> 8: 4604 mov r4, r0 a: 4628 mov r0, r5 c: f7ff fffe bl 0 <devfdt_get_addr> 10: 6020 str r0, [r4, #0] 12: 2000 movs r0, #0 14: bd38 pop {r3, r4, r5, pc}
arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-spl-v2.asm arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o > ../dw-apb-timer-20220105-tpl-v2.asm
===========================================================================
On 1/5/22 3:03 PM, Simon Glass wrote:
Hi Johan,
On Tue, 4 Jan 2022 at 19:15, Johan Jonker jbx6244@gmail.com wrote:
The Rockchip rk3066 SoC has 3 dw-apb-timer nodes. U-boot is compiled with OF_PLATDATA TPL/SPL options, so add OF_PLATDATA support for the dw-apb-timer. Also change driver name to be able to compile with U-boot scripts. No reset OF_PLATDATA support was added, because the rk3066 nodes don't need/have them.
Signed-off-by: Johan Jonker jbx6244@gmail.com
Changed V2: replace if (IS_ENABLED(OF_REAL)) by #if CONFIG_IS_ENABLED(OF_REAL)
drivers/timer/dw-apb-timer.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
This seems OK but you have included unrelated changes (whitespace) which should go in a separate patch.
OK With whitespace did you mean this or something else?
.of_to_plat = dw_apb_timer_of_to_plat,
.of_to_plat = dw_apb_timer_of_to_plat,Can you use if() instead of #if for the CONFIG_IS_ENABLED(OF_REAL) ?
See comments above. Currently not it seems.
Regards, Simon
participants (2)
-
Johan Jonker -
Simon Glass