[U-Boot] [PATCH 0/6] arm: socfpga: gen5: DM improvements

This series ports more ad-hoc code to DM drivers (reset, clk, timer).
Simon Goldschmidt (6): ddr: socfpga: gen5: constify altera_gen5_sdram_ops arm: socfpga: gen5: increase SPL_SYS_MALLOC_F_LEN timer: dw-apb: add reset handling arm: socfpga: gen5: move initial reset handling to reset driver arm: socfpga: gen5: add readonly clk driver arm: socfpga: gen5: use DM_TIMER for systick
MAINTAINERS | 1 + arch/arm/dts/socfpga-common-u-boot.dtsi | 78 ++++ arch/arm/dts/socfpga.dtsi | 5 + .../dts/socfpga_cyclone5_socrates-u-boot.dtsi | 1 + .../socfpga_cyclone5_socrates_handoff.dtsi | 26 ++ arch/arm/mach-socfpga/Kconfig | 7 +- arch/arm/mach-socfpga/Makefile | 1 - arch/arm/mach-socfpga/reset_manager_gen5.c | 13 - arch/arm/mach-socfpga/spl_gen5.c | 24 +- arch/arm/mach-socfpga/timer.c | 23 -- drivers/clk/altera/Makefile | 1 + drivers/clk/altera/clk-gen5.c | 338 ++++++++++++++++++ drivers/ddr/altera/sdram_gen5.c | 2 +- drivers/timer/dw-apb-timer.c | 18 +- 14 files changed, 483 insertions(+), 55 deletions(-) create mode 100644 arch/arm/dts/socfpga_cyclone5_socrates_handoff.dtsi delete mode 100644 arch/arm/mach-socfpga/timer.c create mode 100644 drivers/clk/altera/clk-gen5.c

Make the function pointer struct const, as it does not need to be writable. This doesn't really change anything other than moving this variable to a different section. No functional change.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
drivers/ddr/altera/sdram_gen5.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/sdram_gen5.c index fcd89b619d..8c8ea19eb9 100644 --- a/drivers/ddr/altera/sdram_gen5.c +++ b/drivers/ddr/altera/sdram_gen5.c @@ -626,7 +626,7 @@ static int altera_gen5_sdram_get_info(struct udevice *dev, return 0; }
-static struct ram_ops altera_gen5_sdram_ops = { +static const struct ram_ops altera_gen5_sdram_ops = { .get_info = altera_gen5_sdram_get_info, };

In preparation to adding more DM based drivers, increase the SPL pre-relocation heap just enough to allow those new drivers to run.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
arch/arm/mach-socfpga/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig index 1d914648e3..9efdcd6f10 100644 --- a/arch/arm/mach-socfpga/Kconfig +++ b/arch/arm/mach-socfpga/Kconfig @@ -13,7 +13,7 @@ config SPL_STACK_R_ADDR default 0x00800000 if TARGET_SOCFPGA_GEN5
config SPL_SYS_MALLOC_F_LEN - default 0x800 if TARGET_SOCFPGA_GEN5 + default 0x1100 if TARGET_SOCFPGA_GEN5
config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE default 0xa2

To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv { - fdt_addr_t regs; + fdt_addr_t regs; + struct reset_ctl_bulk resets; };
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
+ ret = reset_get_bulk(dev, &priv->resets); + if (ret) + dev_warn(dev, "Can't get reset: %d\n", ret); + else + reset_deassert_bulk(&priv->resets); + ret = clk_get_by_index(dev, 0, &clk); if (ret) return ret; @@ -67,6 +75,13 @@ static int dw_apb_timer_ofdata_to_platdata(struct udevice *dev) return 0; }
+static int dw_apb_timer_remove(struct udevice *dev) +{ + struct dw_apb_timer_priv *priv = dev_get_priv(dev); + + return reset_release_bulk(&priv->resets); +} + static const struct timer_ops dw_apb_timer_ops = { .get_count = dw_apb_timer_get_count, }; @@ -83,5 +98,6 @@ U_BOOT_DRIVER(dw_apb_timer) = { .probe = dw_apb_timer_probe, .of_match = dw_apb_timer_ids, .ofdata_to_platdata = dw_apb_timer_ofdata_to_platdata, + .remove = dw_apb_timer_remove, .priv_auto_alloc_size = sizeof(struct dw_apb_timer_priv), };

On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv {
- fdt_addr_t regs;
- fdt_addr_t regs;
- struct reset_ctl_bulk resets;
};
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
- ret = reset_get_bulk(dev, &priv->resets);
- if (ret)
dev_warn(dev, "Can't get reset: %d\n", ret);
Shouldn't this be printed by the subsystem ?
[...]

On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv {
fdt_addr_t regs;
fdt_addr_t regs;
struct reset_ctl_bulk resets;
};
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
ret = reset_get_bulk(dev, &priv->resets);
if (ret)
dev_warn(dev, "Can't get reset: %d\n", ret);
Shouldn't this be printed by the subsystem ?
I don't think it's printed by the subsystem. Plus I don't know if it's a warning for all drivers? I also haven't really thought about that, as I just copied the reset handling code from another driver...
Regards, Simon

On 7/24/19 9:43 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv {
fdt_addr_t regs;
fdt_addr_t regs;
struct reset_ctl_bulk resets;
};
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
ret = reset_get_bulk(dev, &priv->resets);
if (ret)
dev_warn(dev, "Can't get reset: %d\n", ret);
Shouldn't this be printed by the subsystem ?
I don't think it's printed by the subsystem. Plus I don't know if it's a warning for all drivers? I also haven't really thought about that, as I just copied the reset handling code from another driver...
Hmmmm, I guess we cannot operate without clock anyway, so it should be dev_err(). And I guess it's indeed not a subsystem print afterall.

On Wed, Jul 24, 2019 at 9:48 AM Marek Vasut marex@denx.de wrote:
On 7/24/19 9:43 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv {
fdt_addr_t regs;
fdt_addr_t regs;
struct reset_ctl_bulk resets;
};
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
ret = reset_get_bulk(dev, &priv->resets);
if (ret)
dev_warn(dev, "Can't get reset: %d\n", ret);
Shouldn't this be printed by the subsystem ?
I don't think it's printed by the subsystem. Plus I don't know if it's a warning for all drivers? I also haven't really thought about that, as I just copied the reset handling code from another driver...
Hmmmm, I guess we cannot operate without clock anyway, so it should be dev_err(). And I guess it's indeed not a subsystem print afterall.
Well, for gen5, it would be dev_err. I'm not sure about arria10 and stratix10 though? I think at least stratix10 doesn't have a clock driver, yet.
Regards, Simon

On 7/24/19 8:09 PM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:48 AM Marek Vasut marex@denx.de wrote:
On 7/24/19 9:43 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv {
fdt_addr_t regs;
fdt_addr_t regs;
struct reset_ctl_bulk resets;
};
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
ret = reset_get_bulk(dev, &priv->resets);
if (ret)
dev_warn(dev, "Can't get reset: %d\n", ret);
Shouldn't this be printed by the subsystem ?
I don't think it's printed by the subsystem. Plus I don't know if it's a warning for all drivers? I also haven't really thought about that, as I just copied the reset handling code from another driver...
Hmmmm, I guess we cannot operate without clock anyway, so it should be dev_err(). And I guess it's indeed not a subsystem print afterall.
Well, for gen5, it would be dev_err. I'm not sure about arria10 and stratix10 though? I think at least stratix10 doesn't have a clock driver, yet.
But it does have reset driver, doesn't it ?

On Thu, Jul 25, 2019 at 1:08 PM Marek Vasut marex@denx.de wrote:
On 7/24/19 8:09 PM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:48 AM Marek Vasut marex@denx.de wrote:
On 7/24/19 9:43 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
To use this timer on socfpga as system tick, it needs to take itself out of reset.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
drivers/timer/dw-apb-timer.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 86312b8dc7..fad22be8c9 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 <reset.h> #include <timer.h>
#include <asm/io.h> @@ -18,7 +19,8 @@ #define DW_APB_CTRL 0x8
struct dw_apb_timer_priv {
fdt_addr_t regs;
fdt_addr_t regs;
struct reset_ctl_bulk resets;
};
static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) @@ -42,6 +44,12 @@ static int dw_apb_timer_probe(struct udevice *dev) struct clk clk; int ret;
ret = reset_get_bulk(dev, &priv->resets);
if (ret)
dev_warn(dev, "Can't get reset: %d\n", ret);
Shouldn't this be printed by the subsystem ?
I don't think it's printed by the subsystem. Plus I don't know if it's a warning for all drivers? I also haven't really thought about that, as I just copied the reset handling code from another driver...
Hmmmm, I guess we cannot operate without clock anyway, so it should be dev_err(). And I guess it's indeed not a subsystem print afterall.
Well, for gen5, it would be dev_err. I'm not sure about arria10 and stratix10 though? I think at least stratix10 doesn't have a clock driver, yet.
But it does have reset driver, doesn't it ?
Yes it does. You confused me with saying "we cannot operate without clock anyway", but this is about reset. So yes, you're right, dev_err() is better.
Regards, Simon

This moves disabling all peripherals from ad-hoc code in arch/arm to the socfpga reset driver.
To do this, DM initialization and UCLASS_RESET probing has to be done earlier in the SPL.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
arch/arm/mach-socfpga/reset_manager_gen5.c | 13 ------------- arch/arm/mach-socfpga/spl_gen5.c | 21 +++++++++------------ 2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c b/arch/arm/mach-socfpga/reset_manager_gen5.c index 9a32f5abfe..34e59b852b 100644 --- a/arch/arm/mach-socfpga/reset_manager_gen5.c +++ b/arch/arm/mach-socfpga/reset_manager_gen5.c @@ -48,19 +48,6 @@ void socfpga_per_reset(u32 reset, int set) clrbits_le32(reg, 1 << RSTMGR_RESET(reset)); }
-/* - * Assert reset on every peripheral but L4WD0. - * Watchdog must be kept intact to prevent glitches - * and/or hangs. - */ -void socfpga_per_reset_all(void) -{ - const u32 l4wd0 = 1 << RSTMGR_RESET(SOCFPGA_RESET(L4WD0)); - - writel(~l4wd0, &reset_manager_base->per_mod_reset); - writel(0xffffffff, &reset_manager_base->per2_mod_reset); -} - #define L3REGS_REMAP_LWHPS2FPGA_MASK 0x10 #define L3REGS_REMAP_HPS2FPGA_MASK 0x08 #define L3REGS_REMAP_OCRAM_MASK 0x01 diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 87b76b47de..1ae8025746 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -84,12 +84,19 @@ void board_init_f(ulong dummy) socfpga_sdram_remap_zero(); socfpga_pl310_clear();
+ ret = spl_early_init(); + if (ret) { + debug("spl_early_init() failed: %d\n", ret); + hang(); + } + debug("Freezing all I/O banks\n"); /* freeze all IO banks */ sys_mgr_frzctrl_freeze_req();
- /* Put everything into reset but L4WD0. */ - socfpga_per_reset_all(); + ret = uclass_get_device(UCLASS_RESET, 0, &dev); + if (ret) + debug("Reset init failed: %d\n", ret);
if (!socfpga_is_booting_from_fpga()) { /* Put FPGA bridges into reset too. */ @@ -130,16 +137,6 @@ void board_init_f(ulong dummy) debug_uart_init(); #endif
- ret = spl_early_init(); - if (ret) { - debug("spl_early_init() failed: %d\n", ret); - hang(); - } - - ret = uclass_get_device(UCLASS_RESET, 0, &dev); - if (ret) - debug("Reset init failed: %d\n", ret); - /* enable console uart printing */ preloader_console_init();

On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This moves disabling all peripherals from ad-hoc code in arch/arm to the socfpga reset driver.
That doesn't seem like what the patch does.
Also, the WDT has to be kept enabled, is that functionality retained ?
To do this, DM initialization and UCLASS_RESET probing has to be done earlier in the SPL.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
[...]

On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This moves disabling all peripherals from ad-hoc code in arch/arm to the socfpga reset driver.
That doesn't seem like what the patch does.
D'oh, you're right. I messed that up during development. In effect, this patch leaves periph reset like it was set by the boot rom.
I'll have to work on v2 for that. Moving this code up is nevertheless required to have the reset driver available for the clock manager etc.
Regards, Simon
Also, the WDT has to be kept enabled, is that functionality retained ?
To do this, DM initialization and UCLASS_RESET probing has to be done earlier in the SPL.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
[...]

This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are: - select DM_CLK for gen5 U-Boot and SPL - add SPL tags to clock nodes in socfpga-common-u-boot.dtsi - adjust socfpga.dtsi to provide missing src selection registers - start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
MAINTAINERS | 1 + arch/arm/dts/socfpga-common-u-boot.dtsi | 70 ++++ arch/arm/dts/socfpga.dtsi | 5 + .../dts/socfpga_cyclone5_socrates-u-boot.dtsi | 1 + .../socfpga_cyclone5_socrates_handoff.dtsi | 26 ++ arch/arm/mach-socfpga/Kconfig | 2 + drivers/clk/altera/Makefile | 1 + drivers/clk/altera/clk-gen5.c | 338 ++++++++++++++++++ 8 files changed, 444 insertions(+) create mode 100644 arch/arm/dts/socfpga_cyclone5_socrates_handoff.dtsi create mode 100644 drivers/clk/altera/clk-gen5.c
diff --git a/MAINTAINERS b/MAINTAINERS index a7d626ffff..b7a9a2671f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -94,6 +94,7 @@ M: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com S: Maintainted T: git https://gitlab.denx.de/u-boot/custodians/u-boot-socfpga.git F: arch/arm/mach-socfpga/ +F: drivers/clk/altera/clk-gen5.c
ARM AMLOGIC SOC SUPPORT M: Neil Armstrong narmstrong@baylibre.com diff --git a/arch/arm/dts/socfpga-common-u-boot.dtsi b/arch/arm/dts/socfpga-common-u-boot.dtsi index 322c858c4b..270ba99a63 100644 --- a/arch/arm/dts/socfpga-common-u-boot.dtsi +++ b/arch/arm/dts/socfpga-common-u-boot.dtsi @@ -7,6 +7,12 @@ /{ soc { u-boot,dm-pre-reloc; + clkmgr@ffd04000 { + u-boot,dm-pre-reloc; + clocks { + u-boot,dm-pre-reloc; + }; + }; }; };
@@ -17,3 +23,67 @@ &sdr { u-boot,dm-pre-reloc; }; + +&osc1 { + u-boot,dm-pre-reloc; +}; + +&osc2 { + u-boot,dm-pre-reloc; +}; + +&f2s_periph_ref_clk { + u-boot,dm-pre-reloc; +}; + +&main_pll { + u-boot,dm-pre-reloc; +}; + +&mainclk { + u-boot,dm-pre-reloc; +}; + +&main_qspi_clk { + u-boot,dm-pre-reloc; +}; + +&main_nand_sdmmc_clk { + u-boot,dm-pre-reloc; +}; + +&periph_pll { + u-boot,dm-pre-reloc; +}; + +&per_qspi_clk { + u-boot,dm-pre-reloc; +}; + +&per_nand_mmc_clk { + u-boot,dm-pre-reloc; +}; + +&per_base_clk { + u-boot,dm-pre-reloc; +}; + +&l4_mp_clk { + u-boot,dm-pre-reloc; +}; + +&l4_sp_clk { + u-boot,dm-pre-reloc; +}; + +&sdmmc_clk { + u-boot,dm-pre-reloc; +}; + +&sdmmc_clk_divided { + u-boot,dm-pre-reloc; +}; + +&qspi_clk { + u-boot,dm-pre-reloc; +}; diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index 51a6a51b53..319334899b 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -332,6 +332,7 @@ compatible = "altr,socfpga-gate-clk"; clocks = <&mainclk>, <&per_base_clk>; div-reg = <0x64 4 3>; + src-reg = <0x70 0 1>; clk-gate = <0x60 2>; };
@@ -340,6 +341,7 @@ compatible = "altr,socfpga-gate-clk"; clocks = <&mainclk>, <&per_base_clk>; div-reg = <0x64 7 3>; + src-reg = <0x70 1 1>; clk-gate = <0x60 3>; };
@@ -453,6 +455,7 @@ #clock-cells = <0>; compatible = "altr,socfpga-gate-clk"; clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>; + src-reg = <0xac 0 2>; clk-gate = <0xa0 8>; clk-phase = <0 135>; }; @@ -469,6 +472,7 @@ #clock-cells = <0>; compatible = "altr,socfpga-gate-clk"; clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>; + src-reg = <0xac 2 2>; clk-gate = <0xa0 9>; };
@@ -491,6 +495,7 @@ #clock-cells = <0>; compatible = "altr,socfpga-gate-clk"; clocks = <&f2s_periph_ref_clk>, <&main_qspi_clk>, <&per_qspi_clk>; + src-reg = <0xac 4 2>; clk-gate = <0xa0 11>; };
diff --git a/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi b/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi index 0a4d54e304..36ae10ab28 100644 --- a/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi +++ b/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi @@ -7,6 +7,7 @@ */
#include "socfpga-common-u-boot.dtsi" +#include "socfpga_cyclone5_socrates_handoff.dtsi"
/{ aliases { diff --git a/arch/arm/dts/socfpga_cyclone5_socrates_handoff.dtsi b/arch/arm/dts/socfpga_cyclone5_socrates_handoff.dtsi new file mode 100644 index 0000000000..e6f19f2653 --- /dev/null +++ b/arch/arm/dts/socfpga_cyclone5_socrates_handoff.dtsi @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + *<auto-generated> + * This code was generated by a tool based on + * handoffs from both Qsys and Quartus. + * + * Changes to this file may be lost if + * the code is regenerated. + *</auto-generated> + */ + +&osc1 { + clock-frequency = <25000000>; +}; + +&osc2 { + clock-frequency = <25000000>; +}; + +&f2s_periph_ref_clk { + clock-frequency = <0>; +}; + +&f2s_sdram_ref_clk { + clock-frequency = <0>; +}; diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig index 9efdcd6f10..81052f27d5 100644 --- a/arch/arm/mach-socfpga/Kconfig +++ b/arch/arm/mach-socfpga/Kconfig @@ -53,7 +53,9 @@ config TARGET_SOCFPGA_CYCLONE5
config TARGET_SOCFPGA_GEN5 bool + select CLK select SPL_ALTERA_SDRAM + select SPL_CLK if SPL imply FPGA_SOCFPGA imply SPL_SIZE_LIMIT_SUBTRACT_GD imply SPL_SIZE_LIMIT_SUBTRACT_MALLOC diff --git a/drivers/clk/altera/Makefile b/drivers/clk/altera/Makefile index a3ae8b24b0..d0664c6ad5 100644 --- a/drivers/clk/altera/Makefile +++ b/drivers/clk/altera/Makefile @@ -4,3 +4,4 @@ #
obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += clk-arria10.o +obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += clk-gen5.o diff --git a/drivers/clk/altera/clk-gen5.c b/drivers/clk/altera/clk-gen5.c new file mode 100644 index 0000000000..4c197b81b0 --- /dev/null +++ b/drivers/clk/altera/clk-gen5.c @@ -0,0 +1,338 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Marek Vasut marex@denx.de + */ + +#include <common.h> +#include <asm/io.h> +#include <clk-uclass.h> +#include <dm.h> +#include <dm/lists.h> +#include <dm/util.h> + +#include <asm/arch/clock_manager.h> + +enum socfpga_gen5_clk_type { + SOCFPGA_GEN5_CLK_MAIN_PLL, + SOCFPGA_GEN5_CLK_PER_PLL, + SOCFPGA_GEN5_CLK_PERIP_CLK, + SOCFPGA_GEN5_CLK_GATE_CLK, + SOCFPGA_GEN5_CLK_UNKNOWN_CLK, +}; + +struct socfpga_gen5_clk_platdata { + enum socfpga_gen5_clk_type type; + struct clk_bulk clks; + u32 regs; + /* Fixed divider */ + u16 fix_div; + /* Control register */ + u16 ctl_reg; + /* Divider register */ + u16 div_reg; + u8 div_len; + u8 div_off; + /* Clock gating register */ + u16 gate_reg; + u8 gate_bit; + /* Clock source register */ + u16 src_reg; + u8 src_len; + u8 src_off; +}; + +static int socfpga_gen5_clk_get_upstream(struct clk *clk, struct clk **upclk) +{ + struct socfpga_gen5_clk_platdata *plat = dev_get_platdata(clk->dev); + u32 reg, maxval; + + if (plat->clks.count == 0) + return 0; + + if (plat->src_reg && plat->src_len) { + maxval = 1 << plat->src_len; + if (plat->clks.count <= maxval) { + reg = readl(plat->regs + plat->src_reg); + reg >>= plat->src_off; + reg &= (maxval - 1); + *upclk = &plat->clks.clks[reg]; + return 0; + } + } + + if (plat->clks.count == 1) { + *upclk = &plat->clks.clks[0]; + return 0; + } + + if (!plat->ctl_reg) + return -EINVAL; + + reg = readl(plat->regs + plat->ctl_reg); + + /* Assume PLLs are ON for now */ + if (plat->type == SOCFPGA_GEN5_CLK_MAIN_PLL) { + reg = (reg >> 8) & 0x3; + maxval = 2; + } else if (plat->type == SOCFPGA_GEN5_CLK_PER_PLL) { + reg = (reg >> 8) & 0x3; + maxval = 3; + } else { + reg = (reg >> 16) & 0x7; + maxval = 4; + } + + if (reg > maxval) { + dev_err(clk->dev, "Invalid clock source\n"); + return -EINVAL; + } + + *upclk = &plat->clks.clks[reg]; + return 0; +} + +static int socfpga_gen5_clk_endisable(struct clk *clk, bool enable) +{ + struct socfpga_gen5_clk_platdata *plat = dev_get_platdata(clk->dev); + struct clk *upclk = NULL; + int ret; + + if (!enable && plat->gate_reg) + clrbits_le32(plat->regs + plat->gate_reg, BIT(plat->gate_bit)); + + ret = socfpga_gen5_clk_get_upstream(clk, &upclk); + if (ret) + return ret; + + if (upclk) { + if (enable) + clk_enable(upclk); + else + clk_disable(upclk); + } + + if (enable && plat->gate_reg) + setbits_le32(plat->regs + plat->gate_reg, BIT(plat->gate_bit)); + + return 0; +} + +static int socfpga_gen5_clk_enable(struct clk *clk) +{ + return socfpga_gen5_clk_endisable(clk, true); +} + +static int socfpga_gen5_clk_disable(struct clk *clk) +{ + return socfpga_gen5_clk_endisable(clk, false); +} + +static ulong socfpga_gen5_clk_get_rate(struct clk *clk) +{ + struct socfpga_gen5_clk_platdata *plat = dev_get_platdata(clk->dev); + struct clk *upclk = NULL; + ulong rate, uprate, reg, numer, denom; + int ret; + + ret = socfpga_gen5_clk_get_upstream(clk, &upclk); + if (ret || !upclk) + return 0; + + uprate = clk_get_rate(upclk); + rate = uprate; + + if (plat->type == SOCFPGA_GEN5_CLK_MAIN_PLL) { + reg = readl(plat->regs + plat->ctl_reg); /* VCO */ + numer = (reg & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >> + CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET; + denom = (reg & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >> + CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET; + rate /= denom + 1; + rate *= numer + 1; + } else if (plat->type == SOCFPGA_GEN5_CLK_PER_PLL) { + reg = readl(plat->regs + plat->ctl_reg); /* VCO */ + numer = (reg & CLKMGR_PERPLLGRP_VCO_NUMER_MASK) >> + CLKMGR_PERPLLGRP_VCO_NUMER_OFFSET; + denom = (reg & CLKMGR_PERPLLGRP_VCO_DENOM_MASK) >> + CLKMGR_PERPLLGRP_VCO_DENOM_OFFSET; + rate /= denom + 1; + rate *= numer + 1; + } else { + rate /= plat->fix_div; + + if (plat->fix_div == 1 && plat->ctl_reg) { + reg = readl(plat->regs + plat->ctl_reg); + reg &= 0x1ff; + rate /= reg + 1; + } + + if (plat->div_reg) { + reg = readl(plat->regs + plat->div_reg); + reg >>= plat->div_off; + reg &= (1 << plat->div_len) - 1; + if (plat->type == SOCFPGA_GEN5_CLK_PERIP_CLK) + rate /= reg + 1; + if (plat->type == SOCFPGA_GEN5_CLK_GATE_CLK) + rate /= 1 << reg; + } + } + + return rate; +} + +static const struct clk_ops socfpga_gen5_clk_ops = { + .enable = socfpga_gen5_clk_enable, + .disable = socfpga_gen5_clk_disable, + .get_rate = socfpga_gen5_clk_get_rate, +}; + +static int socfpga_gen5_clk_bind(struct udevice *dev) +{ + const void *fdt = gd->fdt_blob; + int offset = dev_of_offset(dev); + bool pre_reloc_only = !(gd->flags & GD_FLG_RELOC); + const char *name; + int ret; + const char *drv_name; + + for (offset = fdt_first_subnode(fdt, offset); + offset > 0; + offset = fdt_next_subnode(fdt, offset)) { + name = fdt_get_name(fdt, offset, NULL); + if (!name) + return -EINVAL; + + if (!strcmp(name, "clocks")) { + offset = fdt_first_subnode(fdt, offset); + name = fdt_get_name(fdt, offset, NULL); + if (!name) + return -EINVAL; + } + + /* Filter out supported sub-clock */ + if (fdt_node_check_compatible(fdt, offset, + "altr,socfpga-pll-clock") && + fdt_node_check_compatible(fdt, offset, + "altr,socfpga-perip-clk") && + fdt_node_check_compatible(fdt, offset, + "altr,socfpga-gate-clk") && + fdt_node_check_compatible(fdt, offset, "fixed-clock")) + continue; + + if (pre_reloc_only && + !dm_ofnode_pre_reloc(offset_to_ofnode(offset))) + continue; + + if (fdt_node_check_compatible(fdt, offset, "fixed-clock")) + drv_name = "clk-gen5"; + else + drv_name = "fixed_rate_clock"; + + ret = device_bind_driver_to_node(dev, drv_name, name, + offset_to_ofnode(offset), + NULL); + if (ret) + return ret; + } + + return 0; +} + +static int socfpga_gen5_clk_probe(struct udevice *dev) +{ + struct socfpga_gen5_clk_platdata *plat = dev_get_platdata(dev); + const void *fdt = gd->fdt_blob; + int offset = dev_of_offset(dev); + + clk_get_bulk(dev, &plat->clks); + + if (!fdt_node_check_compatible(fdt, offset, + "altr,socfpga-pll-clock")) { + /* Main PLL has 3 upstream clock */ + if (plat->clks.count == 1) + plat->type = SOCFPGA_GEN5_CLK_MAIN_PLL; + else + plat->type = SOCFPGA_GEN5_CLK_PER_PLL; + } else if (!fdt_node_check_compatible(fdt, offset, + "altr,socfpga-perip-clk")) { + plat->type = SOCFPGA_GEN5_CLK_PERIP_CLK; + } else if (!fdt_node_check_compatible(fdt, offset, + "altr,socfpga-gate-clk")) { + plat->type = SOCFPGA_GEN5_CLK_GATE_CLK; + } else { + plat->type = SOCFPGA_GEN5_CLK_UNKNOWN_CLK; + } + + return 0; +} + +static int socfpga_gen5_ofdata_to_platdata(struct udevice *dev) +{ + struct socfpga_gen5_clk_platdata *plat = dev_get_platdata(dev); + struct socfpga_gen5_clk_platdata *pplat; + struct udevice *pdev; + const void *fdt = gd->fdt_blob; + unsigned int divreg[3], gatereg[2], srcreg[3]; + int ret, offset = dev_of_offset(dev); + u32 regs; + + regs = dev_read_u32_default(dev, "reg", 0x0); + + if (!fdt_node_check_compatible(fdt, offset, "altr,clk-mgr")) { + plat->regs = devfdt_get_addr(dev); + } else { + pdev = dev_get_parent(dev); + if (!pdev) + return -ENODEV; + + pplat = dev_get_platdata(pdev); + if (!pplat) + return -EINVAL; + + plat->ctl_reg = regs; + plat->regs = pplat->regs; + } + + plat->type = SOCFPGA_GEN5_CLK_UNKNOWN_CLK; + + plat->fix_div = dev_read_u32_default(dev, "fixed-divider", 1); + + ret = dev_read_u32_array(dev, "src-reg", srcreg, ARRAY_SIZE(srcreg)); + if (!ret) { + plat->src_reg = srcreg[0]; + plat->src_len = srcreg[2]; + plat->src_off = srcreg[1]; + } + + ret = dev_read_u32_array(dev, "div-reg", divreg, ARRAY_SIZE(divreg)); + if (!ret) { + plat->div_reg = divreg[0]; + plat->div_len = divreg[2]; + plat->div_off = divreg[1]; + } + + ret = dev_read_u32_array(dev, "clk-gate", gatereg, ARRAY_SIZE(gatereg)); + if (!ret) { + plat->gate_reg = gatereg[0]; + plat->gate_bit = gatereg[1]; + } + + return 0; +} + +static const struct udevice_id socfpga_gen5_clk_match[] = { + { .compatible = "altr,clk-mgr" }, + {} +}; + +U_BOOT_DRIVER(socfpga_gen5_clk) = { + .name = "clk-gen5", + .id = UCLASS_CLK, + .of_match = socfpga_gen5_clk_match, + .ops = &socfpga_gen5_clk_ops, + .bind = socfpga_gen5_clk_bind, + .probe = socfpga_gen5_clk_probe, + .ofdata_to_platdata = socfpga_gen5_ofdata_to_platdata, + + .platdata_auto_alloc_size = sizeof(struct socfpga_gen5_clk_platdata), +};

On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
These should likely be separate patches then ?
[...]

On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
Regards, Simon
[...]

On 7/24/19 9:45 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
You probably also want to add DM_FLAG_PRE_RELOC and possibly DM_FLAG_OS_PREPARE to tear down some clock before booting Linux.

On Wed, Jul 24, 2019 at 9:51 AM Marek Vasut marex@denx.de wrote:
On 7/24/19 9:45 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
You probably also want to add DM_FLAG_PRE_RELOC and possibly DM_FLAG_OS_PREPARE to tear down some clock before booting Linux.
Ack for DM_FLAG_PRE_RELOC, but I don't know about DM_FLAG_OS_PREPARE, as that could at least bring portability issues with Linux versions that can't control the pripheral clocks (I don't know if that's implemented at all in Linux, yet).
Plus this is currently a readonly driver, so it returns the rate only and doesn't set/enable/disable anything. I've planned that for the future, but it's not there, yet.
Regards, Simon

On 7/24/19 10:24 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:51 AM Marek Vasut marex@denx.de wrote:
On 7/24/19 9:45 AM, Simon Goldschmidt wrote:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
You probably also want to add DM_FLAG_PRE_RELOC and possibly DM_FLAG_OS_PREPARE to tear down some clock before booting Linux.
Ack for DM_FLAG_PRE_RELOC, but I don't know about DM_FLAG_OS_PREPARE, as that could at least bring portability issues with Linux versions that can't control the pripheral clocks (I don't know if that's implemented at all in Linux, yet).
Probably the same issues we see with reset, right ?
Plus this is currently a readonly driver, so it returns the rate only and doesn't set/enable/disable anything. I've planned that for the future, but it's not there, yet.
OK

Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
I'd appreciate your input on this, given the insight to the Altera system you have!
Also, this (and holidays) are delaying this series (I want to put it all together in once series), so this will have to wait until after v2019.10, I guess?
Regards, Simon

On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for
now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
What do we need read-only pinmux driver for ? I would expect pinmux to be more write-only, from the hardware perspective that is.
I'd appreciate your input on this, given the insight to the Altera system you have!
Also, this (and holidays) are delaying this series (I want to put it all together in once series), so this will have to wait until after v2019.10, I guess?
We're already in RC2, so I think so.

Am 15.08.2019 um 18:34 schrieb Marek Vasut:
On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for
now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
What do we need read-only pinmux driver for ? I would expect pinmux to be more write-only, from the hardware perspective that is.
Well, I don't know. I just need a driver that can parse its dts subtree for the config instead of loading from the qts wrapper file. Then this driver needs to be probed at the appropriate place in SPL so that the pins are initialized.
My future plan is then that such a driver could be re-probed after loading some kind of dts overlay matching an FPGA image to be programmed (as that FPGA image can contain different pin settings, e.g. when using loan IO).
I'm just not familiar enough with pinctrl drivers or syscon drivers and could need some input on which direction to take this...
Does a syscon driver for that purpose sound better?
Thanks, Simon
I'd appreciate your input on this, given the insight to the Altera system you have!
Also, this (and holidays) are delaying this series (I want to put it all together in once series), so this will have to wait until after v2019.10, I guess?
We're already in RC2, so I think so.

On 8/15/19 6:57 PM, Simon Goldschmidt wrote:
Am 15.08.2019 um 18:34 schrieb Marek Vasut:
On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote:
This adds clk-gen5 as a readonly DM_CLK driver that can return clocks for the peripherals.
Further changes are:
- select DM_CLK for gen5 U-Boot and SPL
- add SPL tags to clock nodes in socfpga-common-u-boot.dtsi
- adjust socfpga.dtsi to provide missing src selection registers
- start 'handoff.dtsi' file for socrates (conatains clock speeds for
now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
What do we need read-only pinmux driver for ? I would expect pinmux to be more write-only, from the hardware perspective that is.
Well, I don't know. I just need a driver that can parse its dts subtree for the config instead of loading from the qts wrapper file. Then this driver needs to be probed at the appropriate place in SPL so that the pins are initialized.
Sounds more like misc driver or something along those lines.
My future plan is then that such a driver could be re-probed after loading some kind of dts overlay matching an FPGA image to be programmed (as that FPGA image can contain different pin settings, e.g. when using loan IO).
But then it becomes a PINMUX driver.
I'm just not familiar enough with pinctrl drivers or syscon drivers and could need some input on which direction to take this...
Does a syscon driver for that purpose sound better?
Isn't syscon driver for system controllers, which provide e.g. regmaps to subdevices ? I think the altera pinmux shouldn't be syscon.

Am 15.08.2019 um 19:07 schrieb Marek Vasut:
On 8/15/19 6:57 PM, Simon Goldschmidt wrote:
Am 15.08.2019 um 18:34 schrieb Marek Vasut:
On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote:
On 7/23/19 10:27 PM, Simon Goldschmidt wrote: > This adds clk-gen5 as a readonly DM_CLK driver that can return > clocks for > the peripherals. > > Further changes are: > - select DM_CLK for gen5 U-Boot and SPL > - add SPL tags to clock nodes in socfpga-common-u-boot.dtsi > - adjust socfpga.dtsi to provide missing src selection registers > - start 'handoff.dtsi' file for socrates (conatains clock speeds for > now)
These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
What do we need read-only pinmux driver for ? I would expect pinmux to be more write-only, from the hardware perspective that is.
Well, I don't know. I just need a driver that can parse its dts subtree for the config instead of loading from the qts wrapper file. Then this driver needs to be probed at the appropriate place in SPL so that the pins are initialized.
Sounds more like misc driver or something along those lines.
Hmm, probing UCLASS_MISC in SPL to get the pinmux initializes sounds a bit strange, but that's probably OK.
My future plan is then that such a driver could be re-probed after loading some kind of dts overlay matching an FPGA image to be programmed (as that FPGA image can contain different pin settings, e.g. when using loan IO).
But then it becomes a PINMUX driver.
Well, what I'm writing *is* a writeonly driver controlling the pins. However, since we don't know anything about the iocsr part, we can't implement all the functions in 'struct pinctrl_ops' (just write the given scan chain and that's it). I think PINMUX would fit best, but see below...
I'm just not familiar enough with pinctrl drivers or syscon drivers and could need some input on which direction to take this...
Does a syscon driver for that purpose sound better?
Isn't syscon driver for system controllers, which provide e.g. regmaps to subdevices ? I think the altera pinmux shouldn't be syscon.
The thing is that 'sysmgr' already *is* a syscon driver: it provides access to various control bits (e.g. used by the ethernet driver in Linux) but also pinmux registers.
Of course, I could add "scanmgr@0xfff02000" as a new node in the devicetree that would be the PINMUX driver which accesses the pinmux registers in sysmgr via sysycon... That would keep sysmgr's syscon behaviour working (for other drivers).
Regards, Simon

On 8/15/19 7:42 PM, Simon Goldschmidt wrote:
Am 15.08.2019 um 19:07 schrieb Marek Vasut:
On 8/15/19 6:57 PM, Simon Goldschmidt wrote:
Am 15.08.2019 um 18:34 schrieb Marek Vasut:
On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt:
On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote: > > On 7/23/19 10:27 PM, Simon Goldschmidt wrote: >> This adds clk-gen5 as a readonly DM_CLK driver that can return >> clocks for >> the peripherals. >> >> Further changes are: >> - select DM_CLK for gen5 U-Boot and SPL >> - add SPL tags to clock nodes in socfpga-common-u-boot.dtsi >> - adjust socfpga.dtsi to provide missing src selection registers >> - start 'handoff.dtsi' file for socrates (conatains clock speeds >> for >> now) > > These should likely be separate patches then ?
Well, in the end, yes. Especially the handoff.dtsi is required for *all* socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' script to generate it.
I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
What do we need read-only pinmux driver for ? I would expect pinmux to be more write-only, from the hardware perspective that is.
Well, I don't know. I just need a driver that can parse its dts subtree for the config instead of loading from the qts wrapper file. Then this driver needs to be probed at the appropriate place in SPL so that the pins are initialized.
Sounds more like misc driver or something along those lines.
Hmm, probing UCLASS_MISC in SPL to get the pinmux initializes sounds a bit strange, but that's probably OK.
Well it's kinda pinmux, but not really. It's not like you can specify, in DT, how to program a pin or a range of pins either. But maybe PINCTRL uclass with some really rudimentary driver is OK.
I am not sure myself to be honest.
My future plan is then that such a driver could be re-probed after loading some kind of dts overlay matching an FPGA image to be programmed (as that FPGA image can contain different pin settings, e.g. when using loan IO).
But then it becomes a PINMUX driver.
Well, what I'm writing *is* a writeonly driver controlling the pins. However, since we don't know anything about the iocsr part, we can't implement all the functions in 'struct pinctrl_ops' (just write the given scan chain and that's it). I think PINMUX would fit best, but see below...
Go for it then :)
I'm just not familiar enough with pinctrl drivers or syscon drivers and could need some input on which direction to take this...
Does a syscon driver for that purpose sound better?
Isn't syscon driver for system controllers, which provide e.g. regmaps to subdevices ? I think the altera pinmux shouldn't be syscon.
The thing is that 'sysmgr' already *is* a syscon driver: it provides access to various control bits (e.g. used by the ethernet driver in Linux) but also pinmux registers.
Maybe you can have a pinctrl driver that is a consumer of the syscon interface ?
Of course, I could add "scanmgr@0xfff02000" as a new node in the devicetree that would be the PINMUX driver which accesses the pinmux registers in sysmgr via sysycon... That would keep sysmgr's syscon behaviour working (for other drivers).
Regards, Simon

Marek Vasut marex@denx.de schrieb am Fr., 16. Aug. 2019, 13:09:
On 8/15/19 7:42 PM, Simon Goldschmidt wrote:
Am 15.08.2019 um 19:07 schrieb Marek Vasut:
On 8/15/19 6:57 PM, Simon Goldschmidt wrote:
Am 15.08.2019 um 18:34 schrieb Marek Vasut:
On 8/15/19 6:22 PM, Simon Goldschmidt wrote:
Hi Marek,
Am 24.07.2019 um 09:45 schrieb Simon Goldschmidt: > On Wed, Jul 24, 2019 at 9:31 AM Marek Vasut marex@denx.de wrote: >> >> On 7/23/19 10:27 PM, Simon Goldschmidt wrote: >>> This adds clk-gen5 as a readonly DM_CLK driver that can return >>> clocks for >>> the peripherals. >>> >>> Further changes are: >>> - select DM_CLK for gen5 U-Boot and SPL >>> - add SPL tags to clock nodes in socfpga-common-u-boot.dtsi >>> - adjust socfpga.dtsi to provide missing src selection registers >>> - start 'handoff.dtsi' file for socrates (conatains clock speeds >>> for >>> now) >> >> These should likely be separate patches then ? > > Well, in the end, yes. Especially the handoff.dtsi is required for > *all* > socfpga_gen5 boards, so I'll need to adapt the 'qts-filter.sh' > script to > generate it. > > I'll change that script and separate these patches in v2.
I'm in the process of moving all of the 'qts' header files to devicetree handoff.dtsi files. CLK and DDR are already working (pinmux/iocsr not yet) - but I need to work a bit on DM memory consumption.
So now I'm faced with a question: in which driver do I implement the pinmux control? From a DM point of view, it could be a UCLASS_PINCTRL driver in 'drivers/pinctrl', but since it's more or less read-only unless we'd get more details about the hardware, I'm a bit hesistant to do it that way.
Also, the registers are in 'sysmgr', which is handled by the standard "syscon" driver right now, so it could well get a UCLASS_SYSCON driver?
What do we need read-only pinmux driver for ? I would expect pinmux to be more write-only, from the hardware perspective that is.
Well, I don't know. I just need a driver that can parse its dts subtree for the config instead of loading from the qts wrapper file. Then this driver needs to be probed at the appropriate place in SPL so that the pins are initialized.
Sounds more like misc driver or something along those lines.
Hmm, probing UCLASS_MISC in SPL to get the pinmux initializes sounds a bit strange, but that's probably OK.
Well it's kinda pinmux, but not really. It's not like you can specify, in DT, how to program a pin or a range of pins either. But maybe PINCTRL uclass with some really rudimentary driver is OK.
I am not sure myself to be honest.
My future plan is then that such a driver could be re-probed after loading some kind of dts overlay matching an FPGA image to be
programmed
(as that FPGA image can contain different pin settings, e.g. when using loan IO).
But then it becomes a PINMUX driver.
Well, what I'm writing *is* a writeonly driver controlling the pins. However, since we don't know anything about the iocsr part, we can't implement all the functions in 'struct pinctrl_ops' (just write the given scan chain and that's it). I think PINMUX would fit best, but see below...
Go for it then :)
I'm just not familiar enough with pinctrl drivers or syscon drivers and could need some input on which direction to take this...
Does a syscon driver for that purpose sound better?
Isn't syscon driver for system controllers, which provide e.g. regmaps to subdevices ? I think the altera pinmux shouldn't be syscon.
The thing is that 'sysmgr' already *is* a syscon driver: it provides access to various control bits (e.g. used by the ethernet driver in Linux) but also pinmux registers.
Maybe you can have a pinctrl driver that is a consumer of the syscon interface ?
Right, I'll add such a pinctrl as a driver for the scanmgr (which is not yet present in the devicetree) which will use the sysmgr pins via syscon.
Thanks for sharing your thoughts on this!
Regards, Simon
Of course, I could add "scanmgr@0xfff02000" as a new node in the devicetree that would be the PINMUX driver which accesses the pinmux registers in sysmgr via sysycon... That would keep sysmgr's syscon behaviour working (for other drivers).
Regards, Simon
-- Best regards, Marek Vasut

This removes the ad-hoc timer code in arch.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
arch/arm/dts/socfpga-common-u-boot.dtsi | 8 ++++++++ arch/arm/mach-socfpga/Kconfig | 3 +++ arch/arm/mach-socfpga/Makefile | 1 - arch/arm/mach-socfpga/spl_gen5.c | 3 --- arch/arm/mach-socfpga/timer.c | 23 ----------------------- 5 files changed, 11 insertions(+), 27 deletions(-) delete mode 100644 arch/arm/mach-socfpga/timer.c
diff --git a/arch/arm/dts/socfpga-common-u-boot.dtsi b/arch/arm/dts/socfpga-common-u-boot.dtsi index 270ba99a63..e0305d7fc7 100644 --- a/arch/arm/dts/socfpga-common-u-boot.dtsi +++ b/arch/arm/dts/socfpga-common-u-boot.dtsi @@ -5,6 +5,10 @@ * Copyright (c) 2019 Simon Goldschmidt */ /{ + chosen { + tick-timer = &timer2; + }; + soc { u-boot,dm-pre-reloc; clkmgr@ffd04000 { @@ -16,6 +20,10 @@ }; };
+&timer2 { + u-boot,dm-pre-reloc; +}; + &rst { u-boot,dm-pre-reloc; }; diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig index 81052f27d5..518ac8ec4a 100644 --- a/arch/arm/mach-socfpga/Kconfig +++ b/arch/arm/mach-socfpga/Kconfig @@ -56,6 +56,9 @@ config TARGET_SOCFPGA_GEN5 select CLK select SPL_ALTERA_SDRAM select SPL_CLK if SPL + select SPL_TIMER if SPL + select TIMER + imply DESIGNWARE_APB_TIMER imply FPGA_SOCFPGA imply SPL_SIZE_LIMIT_SUBTRACT_GD imply SPL_SIZE_LIMIT_SUBTRACT_MALLOC diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile index e66720447f..3b839c9ffd 100644 --- a/arch/arm/mach-socfpga/Makefile +++ b/arch/arm/mach-socfpga/Makefile @@ -16,7 +16,6 @@ obj-y += misc_gen5.o obj-y += reset_manager_gen5.o obj-y += scan_manager.o obj-y += system_manager_gen5.o -obj-y += timer.o obj-y += wrap_pll_config.o obj-y += fpga_manager.o endif diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 1ae8025746..7a67f538d8 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -103,9 +103,6 @@ void board_init_f(ulong dummy) socfpga_bridges_reset(1); }
- socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); - timer_init(); - debug("Reconfigure Clock Manager\n"); /* reconfigure the PLLs */ if (cm_basic_init(cm_default_cfg)) diff --git a/arch/arm/mach-socfpga/timer.c b/arch/arm/mach-socfpga/timer.c deleted file mode 100644 index f1c0262ae5..0000000000 --- a/arch/arm/mach-socfpga/timer.c +++ /dev/null @@ -1,23 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright (C) 2012 Altera Corporation <www.altera.com> - */ - -#include <common.h> -#include <asm/io.h> -#include <asm/arch/timer.h> - -#define TIMER_LOAD_VAL 0xFFFFFFFF - -static const struct socfpga_timer *timer_base = (void *)CONFIG_SYS_TIMERBASE; - -/* - * Timer initialization - */ -int timer_init(void) -{ - writel(TIMER_LOAD_VAL, &timer_base->load_val); - writel(TIMER_LOAD_VAL, &timer_base->curr_val); - writel(readl(&timer_base->ctrl) | 0x3, &timer_base->ctrl); - return 0; -}

Am 23.07.2019 um 22:27 schrieb Simon Goldschmidt:
This series ports more ad-hoc code to DM drivers (reset, clk, timer).
This is how far I got with DM/DTS "conversion" for now. Converting from "board/qts/*.h" to DTS is still pending since I yet failed to find a solution that allows FPGA images to define different clock or I/O requirements without requiring Linux drivers for clock or pinmux...
Converting from "board/qts/*.h" to U-Boot devicetree is probably not too hard, but reconfiguration after configuring the FPGA (e.g. from FIT) requires more work.
BTW, we're now quite short on SRAM in SPL. Next steps on this platform might require to separate "boot from MMC" and "boot from QSPI" to reduce SRAM usage.
Regards, Simon
Simon Goldschmidt (6): ddr: socfpga: gen5: constify altera_gen5_sdram_ops arm: socfpga: gen5: increase SPL_SYS_MALLOC_F_LEN timer: dw-apb: add reset handling arm: socfpga: gen5: move initial reset handling to reset driver arm: socfpga: gen5: add readonly clk driver arm: socfpga: gen5: use DM_TIMER for systick
MAINTAINERS | 1 + arch/arm/dts/socfpga-common-u-boot.dtsi | 78 ++++ arch/arm/dts/socfpga.dtsi | 5 + .../dts/socfpga_cyclone5_socrates-u-boot.dtsi | 1 + .../socfpga_cyclone5_socrates_handoff.dtsi | 26 ++ arch/arm/mach-socfpga/Kconfig | 7 +- arch/arm/mach-socfpga/Makefile | 1 - arch/arm/mach-socfpga/reset_manager_gen5.c | 13 - arch/arm/mach-socfpga/spl_gen5.c | 24 +- arch/arm/mach-socfpga/timer.c | 23 -- drivers/clk/altera/Makefile | 1 + drivers/clk/altera/clk-gen5.c | 338 ++++++++++++++++++ drivers/ddr/altera/sdram_gen5.c | 2 +- drivers/timer/dw-apb-timer.c | 18 +- 14 files changed, 483 insertions(+), 55 deletions(-) create mode 100644 arch/arm/dts/socfpga_cyclone5_socrates_handoff.dtsi delete mode 100644 arch/arm/mach-socfpga/timer.c create mode 100644 drivers/clk/altera/clk-gen5.c
participants (2)
-
Marek Vasut
-
Simon Goldschmidt