[PATCH v7 0/4] add TCB driver for sama5d2

[PATCH v7 0/4] add TCB driver for sama5d2

[Clément Léger]

When booting under OP-TEE, the SYSC is secured which means the PIT is
also not accessible by non-secure world. The TCB 0 however is always
available for non-secure world and thus can be used.
This series add a TCB driver and enables it in sama5d2 configs.

---
Changes in v7:
- Keep the exact same Linux aic node definition
- Fixed a few constant definition to use (1 << x)

Changes in V6:
- Fix node ordering in device tree
- Disable CONFIG_PIT_TIMER in sama5d2 configuration file
- Fixed mach-at91/armv7/Makefile to allow building with TCB

Changes in v5:
- Fixed missing include in .dts file for IRQ defines
- Add aic interrupt controller node to avoid DTC warnings

Changes in v4:
- Add missing interrupts property to tcb node

Changes in v3:
- Remove useless defines
- Add printf in case of unsupported timers specified in device-tree
- Add "syscon" compatible to tcb node

Changes in v2:
- Reworked driver to use existing Linux bindings

Clément Léger (4):
  timer: atmel_tcb_timer: add atmel_tcb driver
  ARM: dts: at91: sama5d2: add AIC node
  ARM: dts: at91: sama5d2: add TCB node
  configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

 MAINTAINERS                                  |   1 +
 arch/arm/dts/sama5d2.dtsi                    |  25 +++
 arch/arm/mach-at91/armv7/Makefile            |   2 +
 configs/sama5d2_icp_mmc_defconfig            |   2 +-
 configs/sama5d2_ptc_ek_mmc_defconfig         |   2 +-
 configs/sama5d2_ptc_ek_nandflash_defconfig   |   2 +-
 configs/sama5d2_xplained_emmc_defconfig      |   2 +-
 configs/sama5d2_xplained_mmc_defconfig       |   2 +-
 configs/sama5d2_xplained_qspiflash_defconfig |   2 +-
 configs/sama5d2_xplained_spiflash_defconfig  |   2 +-
 drivers/timer/Kconfig                        |   8 +
 drivers/timer/Makefile                       |   1 +
 drivers/timer/atmel_tcb_timer.c              | 154 +++++++++++++++++++
 13 files changed, 198 insertions(+), 7 deletions(-)
 create mode 100644 drivers/timer/atmel_tcb_timer.c

-- 
2.34.1

[PATCH v7 1/4] timer: atmel_tcb_timer: add atmel_tcb driver

[Clément Léger]

Add a driver for the timer counter block that can be found on sama5d2.
This driver will be used when booting under OP-TEE since the pit timer
which is part of the SYSC is secured. Channel 1 & 2 are configured to
be chained together which allows to have a 64bits counter.

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 MAINTAINERS                       |   1 +
 arch/arm/mach-at91/armv7/Makefile |   2 +
 drivers/timer/Kconfig             |   8 ++
 drivers/timer/Makefile            |   1 +
 drivers/timer/atmel_tcb_timer.c   | 154 ++++++++++++++++++++++++++++++
 5 files changed, 166 insertions(+)
 create mode 100644 drivers/timer/atmel_tcb_timer.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0f39bc6bc9..e30389dc3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -352,6 +352,7 @@ F:	arch/arm/mach-at91/
 F:	board/atmel/
 F:	drivers/cpu/at91_cpu.c
 F:	drivers/misc/microchip_flexcom.c
+F:	drivers/timer/atmel_tcb_timer.c
 F:	include/dt-bindings/mfd/atmel-flexcom.h
 F:	drivers/timer/mchp-pit64b-timer.c
 
diff --git a/arch/arm/mach-at91/armv7/Makefile b/arch/arm/mach-at91/armv7/Makefile
index 246050b67b..f395b55c3d 100644
--- a/arch/arm/mach-at91/armv7/Makefile
+++ b/arch/arm/mach-at91/armv7/Makefile
@@ -14,9 +14,11 @@ obj-y += cpu.o
 ifndef CONFIG_$(SPL_TPL_)SYSRESET
 obj-y	+= reset.o
 endif
+ifneq ($(CONFIG_ATMEL_TCB_TIMER),y)
 ifneq ($(CONFIG_ATMEL_PIT_TIMER),y)
 ifneq ($(CONFIG_MCHP_PIT64B_TIMER),y)
 # old non-DM timer driver
 obj-y += timer.o
 endif
 endif
+endif
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index 8913142654..8fad59b81a 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -96,6 +96,14 @@ config ATMEL_PIT_TIMER
 	  it is designed to offer maximum accuracy and efficient management,
 	  even for systems with long response time.
 
+config ATMEL_TCB_TIMER
+	bool "Atmel timer counter support"
+	depends on TIMER
+	depends on ARCH_AT91
+	help
+	  Select this to enable the use of the timer counter as a monotonic
+	  counter.
+
 config CADENCE_TTC_TIMER
 	bool "Cadence TTC (Triple Timer Counter)"
 	depends on TIMER
diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
index e2bd530eb0..58da6c1e84 100644
--- a/drivers/timer/Makefile
+++ b/drivers/timer/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ARC_TIMER)	+= arc_timer.o
 obj-$(CONFIG_AST_TIMER)	+= ast_timer.o
 obj-$(CONFIG_ATCPIT100_TIMER) += atcpit100_timer.o
 obj-$(CONFIG_ATMEL_PIT_TIMER) += atmel_pit_timer.o
+obj-$(CONFIG_ATMEL_TCB_TIMER) += atmel_tcb_timer.o
 obj-$(CONFIG_CADENCE_TTC_TIMER)	+= cadence-ttc.o
 obj-$(CONFIG_DESIGNWARE_APB_TIMER)	+= dw-apb-timer.o
 obj-$(CONFIG_MPC83XX_TIMER) += mpc83xx_timer.o
diff --git a/drivers/timer/atmel_tcb_timer.c b/drivers/timer/atmel_tcb_timer.c
new file mode 100644
index 0000000000..70f43d3f73
--- /dev/null
+++ b/drivers/timer/atmel_tcb_timer.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Microchip Corporation
+ *
+ * Author: Clément Léger <clement.leger@bootlin.com>
+ */
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <timer.h>
+#include <asm/io.h>
+#include <linux/bitops.h>
+
+#define TCB_CHAN(chan)		((chan) * 0x40)
+
+#define TCB_CCR(chan)		(0x0 + TCB_CHAN(chan))
+#define  TCB_CCR_CLKEN		(1 << 0)
+
+#define TCB_CMR(chan)		(0x4 + TCB_CHAN(chan))
+#define  TCB_CMR_WAVE		(1 << 15)
+#define  TCB_CMR_TIMER_CLOCK1	0
+#define  TCB_CMR_XC1		6
+#define  TCB_CMR_ACPA_SET	(1 << 16)
+#define  TCB_CMR_ACPC_CLEAR	(2 << 18)
+
+#define TCB_CV(chan)		(0x10 + TCB_CHAN(chan))
+
+#define TCB_RA(chan)		(0x14 + TCB_CHAN(chan))
+#define TCB_RC(chan)		(0x1c + TCB_CHAN(chan))
+
+#define TCB_IDR(chan)		(0x28 + TCB_CHAN(chan))
+
+#define TCB_BCR			0xc0
+#define  TCB_BCR_SYNC		(1 << 0)
+
+#define TCB_BMR			0xc4
+#define  TCB_BMR_TC1XC1S_TIOA0	(2 << 2)
+
+#define TCB_WPMR		0xe4
+#define  TCB_WPMR_WAKEY		0x54494d
+
+struct atmel_tcb_plat {
+	void __iomem *base;
+};
+
+static u64 atmel_tcb_get_count(struct udevice *dev)
+{
+	struct atmel_tcb_plat *plat = dev_get_plat(dev);
+	u64 cv0 = 0;
+	u64 cv1 = 0;
+
+	do {
+		cv1 = readl(plat->base + TCB_CV(1));
+		cv0 = readl(plat->base + TCB_CV(0));
+	} while (readl(plat->base + TCB_CV(1)) != cv1);
+
+	cv0 |= cv1 << 32;
+
+	return cv0;
+}
+
+static void atmel_tcb_configure(void __iomem *base)
+{
+	/* Disable write protection */
+	writel(TCB_WPMR_WAKEY, base + TCB_WPMR);
+
+	/* Disable all irqs for both channel 0 & 1 */
+	writel(0xff, base + TCB_IDR(0));
+	writel(0xff, base + TCB_IDR(1));
+
+	/*
+	 * In order to avoid wrapping, use a 64 bit counter by chaining
+	 * two channels.
+	 * Channel 0 is configured to generate a clock on TIOA0 which is cleared
+	 * when reaching 0x80000000 and set when reaching 0.
+	 */
+	writel(TCB_CMR_TIMER_CLOCK1 | TCB_CMR_WAVE | TCB_CMR_ACPA_SET
+		   | TCB_CMR_ACPC_CLEAR, base + TCB_CMR(0));
+	writel(0x80000000, base + TCB_RC(0));
+	writel(0x1, base + TCB_RA(0));
+	writel(TCB_CCR_CLKEN, base + TCB_CCR(0));
+
+	/* Channel 1 is configured to use TIOA0 as input */
+	writel(TCB_CMR_XC1 | TCB_CMR_WAVE, base + TCB_CMR(1));
+	writel(TCB_CCR_CLKEN, base + TCB_CCR(1));
+
+	/* Set XC1 input to be TIOA0 (ie output of Channel 0) */
+	writel(TCB_BMR_TC1XC1S_TIOA0, base + TCB_BMR);
+
+	/* Sync & start all timers */
+	writel(TCB_BCR_SYNC, base + TCB_BCR);
+}
+
+static int atmel_tcb_probe(struct udevice *dev)
+{
+	struct atmel_tcb_plat *plat = dev_get_plat(dev);
+	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct clk clk;
+	ulong clk_rate;
+	int ret;
+
+	if (!device_is_compatible(dev->parent, "atmel,sama5d2-tcb"))
+		return -EINVAL;
+
+	/* Currently, we only support channel 0 and 1 to be chained */
+	if (dev_read_addr_index(dev, 0) != 0 &&
+	    dev_read_addr_index(dev, 1) != 1) {
+		printf("Error: only chained timers 0 and 1 are supported\n");
+		return -EINVAL;
+	}
+
+	ret = clk_get_by_name(dev->parent, "gclk", &clk);
+	if (ret)
+		return -EINVAL;
+
+	clk_rate = clk_get_rate(&clk);
+	if (!clk_rate)
+		return -EINVAL;
+
+	uc_priv->clock_rate = clk_rate;
+
+	atmel_tcb_configure(plat->base);
+
+	return 0;
+}
+
+static int atmel_tcb_of_to_plat(struct udevice *dev)
+{
+	struct atmel_tcb_plat *plat = dev_get_plat(dev);
+
+	plat->base = dev_read_addr_ptr(dev->parent);
+
+	return 0;
+}
+
+static const struct timer_ops atmel_tcb_ops = {
+	.get_count = atmel_tcb_get_count,
+};
+
+static const struct udevice_id atmel_tcb_ids[] = {
+	{ .compatible = "atmel,tcb-timer" },
+	{ }
+};
+
+U_BOOT_DRIVER(atmel_tcb) = {
+	.name = "atmel_tcb",
+	.id = UCLASS_TIMER,
+	.of_match = atmel_tcb_ids,
+	.of_to_plat = atmel_tcb_of_to_plat,
+	.plat_auto = sizeof(struct atmel_tcb_plat),
+	.probe = atmel_tcb_probe,
+	.ops = &atmel_tcb_ops,
+};
-- 
2.34.1

[PATCH v7 2/4] ARM: dts: at91: sama5d2: add AIC node

[Clément Léger]

When using interrupts property, a global interrupt controller needs to
be added to avoid warnings when compiling device-tree:

 arch/arm/dts/at91-sama5d2_xplained.dtb: Warning (interrupts_property):
    /ahb/apb/timer@f800c000: Missing interrupt-parent

Add AIC node as the sama5d2 global interrupt controller.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 arch/arm/dts/sama5d2.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi
index 038cd73c03..7de58e4a1f 100644
--- a/arch/arm/dts/sama5d2.dtsi
+++ b/arch/arm/dts/sama5d2.dtsi
@@ -3,6 +3,7 @@
 / {
 	model = "Atmel SAMA5D2 family SoC";
 	compatible = "atmel,sama5d2";
+	interrupt-parent = <&aic>;
 
 	aliases {
 		spi0 = &spi0;
@@ -762,6 +763,14 @@
 				status = "disabled";
 			};
 
+			aic: interrupt-controller@fc020000 {
+				#interrupt-cells = <3>;
+				compatible = "atmel,sama5d2-aic";
+				interrupt-controller;
+				reg = <0xfc020000 0x200>;
+				atmel,external-irqs = <49>;
+			};
+
 			i2c1: i2c@fc028000 {
 				compatible = "atmel,sama5d2-i2c";
 				reg = <0xfc028000 0x100>;
-- 
2.34.1

[PATCH v7 3/4] ARM: dts: at91: sama5d2: add TCB node

[Clément Léger]

Add the device-tree node to describe the TCB timer.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 arch/arm/dts/sama5d2.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi
index 7de58e4a1f..058009adca 100644
--- a/arch/arm/dts/sama5d2.dtsi
+++ b/arch/arm/dts/sama5d2.dtsi
@@ -1,4 +1,5 @@
 #include "skeleton.dtsi"
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	model = "Atmel SAMA5D2 family SoC";
@@ -639,6 +640,21 @@
 				status = "disabled";
 			};
 
+			tcb0: timer@f800c000 {
+				compatible = "atmel,sama5d2-tcb", "simple-mfd", "syscon";
+				reg = <0xf800c000 0x100>;
+				interrupts = <35 IRQ_TYPE_LEVEL_HIGH 0>;
+				clocks = <&tcb0_clk>, <&tcb0_gclk>, <&clk32k>;
+				clock-names = "t0_clk", "gclk", "slow_clk";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				timer0: timer@0 {
+					compatible = "atmel,tcb-timer";
+					reg = <0>, <1>;
+				};
+			};
+
 			uart0: serial@f801c000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xf801c000 0x100>;
-- 
2.34.1

[PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Clément Léger]

Enable CONFIG_ATMEL_TCB_TIMER and disable CONFIG_ATMEL_PIT_TIMER.
This will allow using the TCB timer instead of the PIT one when running
under OP-TEE.

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 configs/sama5d2_icp_mmc_defconfig            | 2 +-
 configs/sama5d2_ptc_ek_mmc_defconfig         | 2 +-
 configs/sama5d2_ptc_ek_nandflash_defconfig   | 2 +-
 configs/sama5d2_xplained_emmc_defconfig      | 2 +-
 configs/sama5d2_xplained_mmc_defconfig       | 2 +-
 configs/sama5d2_xplained_qspiflash_defconfig | 2 +-
 configs/sama5d2_xplained_spiflash_defconfig  | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/configs/sama5d2_icp_mmc_defconfig b/configs/sama5d2_icp_mmc_defconfig
index 7761a57e0c..c035d7aad8 100644
--- a/configs/sama5d2_icp_mmc_defconfig
+++ b/configs/sama5d2_icp_mmc_defconfig
@@ -82,6 +82,6 @@ CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_ATMEL_USART=y
 CONFIG_TIMER=y
 CONFIG_SPL_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_OF_LIBFDT_OVERLAY=y
 # CONFIG_EFI_LOADER_HII is not set
diff --git a/configs/sama5d2_ptc_ek_mmc_defconfig b/configs/sama5d2_ptc_ek_mmc_defconfig
index 9f458e100b..7cb5ba37f6 100644
--- a/configs/sama5d2_ptc_ek_mmc_defconfig
+++ b/configs/sama5d2_ptc_ek_mmc_defconfig
@@ -71,7 +71,7 @@ CONFIG_DM_SERIAL=y
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_ATMEL_USART=y
 CONFIG_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/sama5d2_ptc_ek_nandflash_defconfig b/configs/sama5d2_ptc_ek_nandflash_defconfig
index 6460ff3dad..667a10e93f 100644
--- a/configs/sama5d2_ptc_ek_nandflash_defconfig
+++ b/configs/sama5d2_ptc_ek_nandflash_defconfig
@@ -71,7 +71,7 @@ CONFIG_DM_SERIAL=y
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_ATMEL_USART=y
 CONFIG_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/sama5d2_xplained_emmc_defconfig b/configs/sama5d2_xplained_emmc_defconfig
index 844a9cde64..72544b65bb 100644
--- a/configs/sama5d2_xplained_emmc_defconfig
+++ b/configs/sama5d2_xplained_emmc_defconfig
@@ -90,7 +90,7 @@ CONFIG_DM_SPI=y
 CONFIG_ATMEL_QSPI=y
 CONFIG_TIMER=y
 CONFIG_SPL_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/sama5d2_xplained_mmc_defconfig b/configs/sama5d2_xplained_mmc_defconfig
index 0de0636587..bcdb84ce2c 100644
--- a/configs/sama5d2_xplained_mmc_defconfig
+++ b/configs/sama5d2_xplained_mmc_defconfig
@@ -92,7 +92,7 @@ CONFIG_DM_SPI=y
 CONFIG_ATMEL_QSPI=y
 CONFIG_TIMER=y
 CONFIG_SPL_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/sama5d2_xplained_qspiflash_defconfig b/configs/sama5d2_xplained_qspiflash_defconfig
index a6e002e59e..c30a90a3e1 100644
--- a/configs/sama5d2_xplained_qspiflash_defconfig
+++ b/configs/sama5d2_xplained_qspiflash_defconfig
@@ -91,7 +91,7 @@ CONFIG_DM_SPI=y
 CONFIG_ATMEL_QSPI=y
 CONFIG_TIMER=y
 CONFIG_SPL_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/sama5d2_xplained_spiflash_defconfig b/configs/sama5d2_xplained_spiflash_defconfig
index 676385fe55..eae54469f8 100644
--- a/configs/sama5d2_xplained_spiflash_defconfig
+++ b/configs/sama5d2_xplained_spiflash_defconfig
@@ -95,7 +95,7 @@ CONFIG_DM_SPI=y
 CONFIG_ATMEL_QSPI=y
 CONFIG_TIMER=y
 CONFIG_SPL_TIMER=y
-CONFIG_ATMEL_PIT_TIMER=y
+CONFIG_ATMEL_TCB_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
-- 
2.34.1

Re: [PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Eugen.Hristev]

On 3/11/22 11:35 AM, Clément Léger wrote:
> Enable CONFIG_ATMEL_TCB_TIMER and disable CONFIG_ATMEL_PIT_TIMER.
> This will allow using the TCB timer instead of the PIT one when running
> under OP-TEE.

Hello Clement,

We have to make sure that the PIT_TIMER was not mandatory for SPL 
operations, and if it was, the TCB_TIMER successfully replaces it.

If we have the PIT_TIMER with the u-boot property
u-boot,dm-pre-reloc;
then the PIT_TIMER is available in the SPL and the SPL works :

RomBOOT
;4▒ls▒K{▒KOp▒C▒▒
<debug_uart>

U-Boot SPL 2022.04-rc4-00204-g5f7086d8a8-dirty (Mar 22 2022 - 13:10:58 
+0200)
SPL: Unsupported Boot Device!
SPL: failed to boot from all boot devices


(well, it fails to boot because I removed the MMC drivers to have it 
smaller than 64k, but it boots)


However, with your patches, even if I add the pre-reloc property to the 
tcb nodes, it fails to boot:

RomBOOT
;4▒Ls▒k{▒kOp▒C▒▒
<debug_uart>
Could not initialize timer (err -22)

Could not initialize timer (err -22)

Could not initialize timer (err -22)

Could not initialize timer (err -22)

...



Could you investigate this please ?

Otherwise we cannot switch to using the TCB if it does not fully replace 
the PIT_TIMER in all the use cases.


Thanks,

Eugen



> 
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---
>   configs/sama5d2_icp_mmc_defconfig            | 2 +-
>   configs/sama5d2_ptc_ek_mmc_defconfig         | 2 +-
>   configs/sama5d2_ptc_ek_nandflash_defconfig   | 2 +-
>   configs/sama5d2_xplained_emmc_defconfig      | 2 +-
>   configs/sama5d2_xplained_mmc_defconfig       | 2 +-
>   configs/sama5d2_xplained_qspiflash_defconfig | 2 +-
>   configs/sama5d2_xplained_spiflash_defconfig  | 2 +-
>   7 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/configs/sama5d2_icp_mmc_defconfig b/configs/sama5d2_icp_mmc_defconfig
> index 7761a57e0c..c035d7aad8 100644
> --- a/configs/sama5d2_icp_mmc_defconfig
> +++ b/configs/sama5d2_icp_mmc_defconfig
> @@ -82,6 +82,6 @@ CONFIG_DEBUG_UART_ANNOUNCE=y
>   CONFIG_ATMEL_USART=y
>   CONFIG_TIMER=y
>   CONFIG_SPL_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_OF_LIBFDT_OVERLAY=y
>   # CONFIG_EFI_LOADER_HII is not set
> diff --git a/configs/sama5d2_ptc_ek_mmc_defconfig b/configs/sama5d2_ptc_ek_mmc_defconfig
> index 9f458e100b..7cb5ba37f6 100644
> --- a/configs/sama5d2_ptc_ek_mmc_defconfig
> +++ b/configs/sama5d2_ptc_ek_mmc_defconfig
> @@ -71,7 +71,7 @@ CONFIG_DM_SERIAL=y
>   CONFIG_DEBUG_UART_ANNOUNCE=y
>   CONFIG_ATMEL_USART=y
>   CONFIG_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_USB=y
>   CONFIG_USB_EHCI_HCD=y
>   CONFIG_USB_STORAGE=y
> diff --git a/configs/sama5d2_ptc_ek_nandflash_defconfig b/configs/sama5d2_ptc_ek_nandflash_defconfig
> index 6460ff3dad..667a10e93f 100644
> --- a/configs/sama5d2_ptc_ek_nandflash_defconfig
> +++ b/configs/sama5d2_ptc_ek_nandflash_defconfig
> @@ -71,7 +71,7 @@ CONFIG_DM_SERIAL=y
>   CONFIG_DEBUG_UART_ANNOUNCE=y
>   CONFIG_ATMEL_USART=y
>   CONFIG_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_USB=y
>   CONFIG_USB_EHCI_HCD=y
>   CONFIG_USB_STORAGE=y
> diff --git a/configs/sama5d2_xplained_emmc_defconfig b/configs/sama5d2_xplained_emmc_defconfig
> index 844a9cde64..72544b65bb 100644
> --- a/configs/sama5d2_xplained_emmc_defconfig
> +++ b/configs/sama5d2_xplained_emmc_defconfig
> @@ -90,7 +90,7 @@ CONFIG_DM_SPI=y
>   CONFIG_ATMEL_QSPI=y
>   CONFIG_TIMER=y
>   CONFIG_SPL_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_USB=y
>   CONFIG_USB_EHCI_HCD=y
>   CONFIG_USB_STORAGE=y
> diff --git a/configs/sama5d2_xplained_mmc_defconfig b/configs/sama5d2_xplained_mmc_defconfig
> index 0de0636587..bcdb84ce2c 100644
> --- a/configs/sama5d2_xplained_mmc_defconfig
> +++ b/configs/sama5d2_xplained_mmc_defconfig
> @@ -92,7 +92,7 @@ CONFIG_DM_SPI=y
>   CONFIG_ATMEL_QSPI=y
>   CONFIG_TIMER=y
>   CONFIG_SPL_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_USB=y
>   CONFIG_USB_EHCI_HCD=y
>   CONFIG_USB_STORAGE=y
> diff --git a/configs/sama5d2_xplained_qspiflash_defconfig b/configs/sama5d2_xplained_qspiflash_defconfig
> index a6e002e59e..c30a90a3e1 100644
> --- a/configs/sama5d2_xplained_qspiflash_defconfig
> +++ b/configs/sama5d2_xplained_qspiflash_defconfig
> @@ -91,7 +91,7 @@ CONFIG_DM_SPI=y
>   CONFIG_ATMEL_QSPI=y
>   CONFIG_TIMER=y
>   CONFIG_SPL_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_USB=y
>   CONFIG_USB_EHCI_HCD=y
>   CONFIG_USB_STORAGE=y
> diff --git a/configs/sama5d2_xplained_spiflash_defconfig b/configs/sama5d2_xplained_spiflash_defconfig
> index 676385fe55..eae54469f8 100644
> --- a/configs/sama5d2_xplained_spiflash_defconfig
> +++ b/configs/sama5d2_xplained_spiflash_defconfig
> @@ -95,7 +95,7 @@ CONFIG_DM_SPI=y
>   CONFIG_ATMEL_QSPI=y
>   CONFIG_TIMER=y
>   CONFIG_SPL_TIMER=y
> -CONFIG_ATMEL_PIT_TIMER=y
> +CONFIG_ATMEL_TCB_TIMER=y
>   CONFIG_USB=y
>   CONFIG_USB_EHCI_HCD=y
>   CONFIG_USB_STORAGE=y
> --
> 2.34.1
> 

Re: [PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Clément Léger]

Le Tue, 22 Mar 2022 11:18:39 +0000,
<Eugen.Hristev@microchip.com> a écrit :

> On 3/11/22 11:35 AM, Clément Léger wrote:
> > Enable CONFIG_ATMEL_TCB_TIMER and disable CONFIG_ATMEL_PIT_TIMER.
> > This will allow using the TCB timer instead of the PIT one when running
> > under OP-TEE.  
> 
> Hello Clement,
> 
> We have to make sure that the PIT_TIMER was not mandatory for SPL 
> operations, and if it was, the TCB_TIMER successfully replaces it.
> 
> If we have the PIT_TIMER with the u-boot property
> u-boot,dm-pre-reloc;
> then the PIT_TIMER is available in the SPL and the SPL works :
> 
> RomBOOT
> ;4▒ls▒K{▒KOp▒C▒▒
> <debug_uart>
> 
> U-Boot SPL 2022.04-rc4-00204-g5f7086d8a8-dirty (Mar 22 2022 - 13:10:58 
> +0200)
> SPL: Unsupported Boot Device!
> SPL: failed to boot from all boot devices
> 
> 
> (well, it fails to boot because I removed the MMC drivers to have it 
> smaller than 64k, but it boots)
> 
> 
> However, with your patches, even if I add the pre-reloc property to the 
> tcb nodes, it fails to boot:
> 
> RomBOOT
> ;4▒Ls▒k{▒kOp▒C▒▒
> <debug_uart>
> Could not initialize timer (err -22)
> 
> Could not initialize timer (err -22)
> 
> Could not initialize timer (err -22)
> 
> Could not initialize timer (err -22)
> 
> ...
> 
> 
> 
> Could you investigate this please ?

Hi Eugen,

Ok, I'll try to debug that !

> 
> Otherwise we cannot switch to using the TCB if it does not fully replace 
> the PIT_TIMER in all the use cases.

Yes, that is completely understandable.

> 
> 
> Thanks,
> 
> Eugen


-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Re: [PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Clément Léger]

Le Tue, 22 Mar 2022 13:35:08 +0100,
Clément Léger <clement.leger@bootlin.com> a écrit :

> Le Tue, 22 Mar 2022 11:18:39 +0000,
> <Eugen.Hristev@microchip.com> a écrit :
> 

[...]

> > 
> > Could not initialize timer (err -22)
> > 
> > ...
> > 
> > 
> > 
> > Could you investigate this please ?  
> 
> Hi Eugen,
> 
> Ok, I'll try to debug that !

Hi Eugen, while debugging it, I found that there was actually one major
flaw. The clock that I used was the GLCK. If only this clock is
enabled, the TCB can not work, the tcb0_clk must also be enabled for the
TCB to start counting. It worked on my side because I forgot that I had
a quirk in OP-TEE that was enabling the clocks before starting U-Boot.
Without that, it actually could not work.

Since the tcb0_clk is needed, I switched the driver to use this one
only and it works well on a "bare" setup (ie without OP-TEE). I even
think it could not work with the tcb0_gclk since this one does not
correctly handles the case when the parent clock is the slow_clk (which
might be the case after booting) and in this case, will return 0 as
rate (this is by the way supported in your CCF branch).

Anyway, I fixed the driver to use the tcb0_clk and thus it should work
much better. I will send a V8 and it should work with U-Boot and the
SPL.

By the way, do you have any documentation which explains how to use
U-Boot SPL on sama5d2_xplained (or other) board ?

Regards,

Clément.

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Re: [PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Eugen.Hristev]

On 3/30/22 6:49 PM, Clément Léger wrote:
> Le Tue, 22 Mar 2022 13:35:08 +0100,
> Clément Léger <clement.leger@bootlin.com> a écrit :
> 
>> Le Tue, 22 Mar 2022 11:18:39 +0000,
>> <Eugen.Hristev@microchip.com> a écrit :
>>
> 
> [...]
> 
>>>
>>> Could not initialize timer (err -22)
>>>
>>> ...
>>>
>>>
>>>
>>> Could you investigate this please ?
>>
>> Hi Eugen,
>>
>> Ok, I'll try to debug that !
> 
> Hi Eugen, while debugging it, I found that there was actually one major
> flaw. The clock that I used was the GLCK. If only this clock is
> enabled, the TCB can not work, the tcb0_clk must also be enabled for the
> TCB to start counting. It worked on my side because I forgot that I had
> a quirk in OP-TEE that was enabling the clocks before starting U-Boot.
> Without that, it actually could not work.
> 
> Since the tcb0_clk is needed, I switched the driver to use this one
> only and it works well on a "bare" setup (ie without OP-TEE). I even
> think it could not work with the tcb0_gclk since this one does not
> correctly handles the case when the parent clock is the slow_clk (which
> might be the case after booting) and in this case, will return 0 as
> rate (this is by the way supported in your CCF branch).
> 
> Anyway, I fixed the driver to use the tcb0_clk and thus it should work
> much better. I will send a V8 and it should work with U-Boot and the
> SPL.
> 
> By the way, do you have any documentation which explains how to use
> U-Boot SPL on sama5d2_xplained (or other) board ?

Hi Clement,

I don't have any documentation at hand, but what I can say in a few 
words is that once you build the SPL, you should have a file 
spl/boot.bin and this binary should replace our stage2 at91 bootloader 
at91bootstrap.

Unlike at91bootstrap, the SPL has all the drivers inside and can boot 
from all supported media, however, SPL will read a dedicated CPU 
register where the stage1 has written some bits representing the index 
of the NVM *used to copy the boot.bin from* . The SPL will then attempt 
to read the u-boot.bin from the same media. (you can alter the file name 
in defconfig ).

What is required for your series is that the SPL still works as it 
worked before your series. (e.g. no regressions with your patches).

Eugen

> 
> Regards,
> 
> Clément.
> 
> --
> Clément Léger,
> Embedded Linux and Kernel engineer at Bootlin
> https://bootlin.com
> 

Re: [PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Clément Léger]

Le Thu, 31 Mar 2022 05:59:02 +0000,
<Eugen.Hristev@microchip.com> a écrit :

> On 3/30/22 6:49 PM, Clément Léger wrote:
> > Le Tue, 22 Mar 2022 13:35:08 +0100,
> > Clément Léger <clement.leger@bootlin.com> a écrit :
> >   
> >> Le Tue, 22 Mar 2022 11:18:39 +0000,
> >> <Eugen.Hristev@microchip.com> a écrit :
> >>  
> > 
> > [...]
> >   
> >>>
> >>> Could not initialize timer (err -22)
> >>>
> >>> ...
> >>>
> >>>
> >>>
> >>> Could you investigate this please ?  
> >>
> >> Hi Eugen,
> >>
> >> Ok, I'll try to debug that !  
> > 
> > Hi Eugen, while debugging it, I found that there was actually one major
> > flaw. The clock that I used was the GLCK. If only this clock is
> > enabled, the TCB can not work, the tcb0_clk must also be enabled for the
> > TCB to start counting. It worked on my side because I forgot that I had
> > a quirk in OP-TEE that was enabling the clocks before starting U-Boot.
> > Without that, it actually could not work.
> > 
> > Since the tcb0_clk is needed, I switched the driver to use this one
> > only and it works well on a "bare" setup (ie without OP-TEE). I even
> > think it could not work with the tcb0_gclk since this one does not
> > correctly handles the case when the parent clock is the slow_clk (which
> > might be the case after booting) and in this case, will return 0 as
> > rate (this is by the way supported in your CCF branch).
> > 
> > Anyway, I fixed the driver to use the tcb0_clk and thus it should work
> > much better. I will send a V8 and it should work with U-Boot and the
> > SPL.
> > 
> > By the way, do you have any documentation which explains how to use
> > U-Boot SPL on sama5d2_xplained (or other) board ?  
> 
> Hi Clement,
> 
> I don't have any documentation at hand, but what I can say in a few 
> words is that once you build the SPL, you should have a file 
> spl/boot.bin and this binary should replace our stage2 at91 bootloader 
> at91bootstrap.
> 
> Unlike at91bootstrap, the SPL has all the drivers inside and can boot 
> from all supported media, however, SPL will read a dedicated CPU 
> register where the stage1 has written some bits representing the index 
> of the NVM *used to copy the boot.bin from* . The SPL will then attempt 
> to read the u-boot.bin from the same media. (you can alter the file name 
> in defconfig ).

Ok, I successfully booted a SPL < 64k from SD card by disabling MMC
support after fixing the TCB driver:

;4��K3K{�KOp�C�0�
U-Boot SPL 2022.04-rc3-00023-gf9da67b41a-dirty (Mar 31 2022 - 09:23:48
+0200)
SPL: Unsupported Boot Device!
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

However, as soon as the size is greater than 64K, it does not boot
anymore. Looks like you already had the same problem. Do you have any
more documentation which would allow me to boot a u-boot.bin file from
this SPL easily ?

By the way, since u-boot,dm-pre-reloc is needed in the device-tree to
be used with the SPL, I will probably add them to my series
(property which by the way is missing for the PIT timer
device-tree node).

Thanks,

Clément

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Re: [PATCH v7 4/4] configs: sama5d2: enable option CONFIG_ATMEL_TCB_TIMER

[Eugen.Hristev]

On 3/31/22 10:40 AM, Clément Léger wrote:
> Le Thu, 31 Mar 2022 05:59:02 +0000,
> <Eugen.Hristev@microchip.com> a écrit :
> 
>> On 3/30/22 6:49 PM, Clément Léger wrote:
>>> Le Tue, 22 Mar 2022 13:35:08 +0100,
>>> Clément Léger <clement.leger@bootlin.com> a écrit :
>>>
>>>> Le Tue, 22 Mar 2022 11:18:39 +0000,
>>>> <Eugen.Hristev@microchip.com> a écrit :
>>>>
>>>
>>> [...]
>>>
>>>>>
>>>>> Could not initialize timer (err -22)
>>>>>
>>>>> ...
>>>>>
>>>>>
>>>>>
>>>>> Could you investigate this please ?
>>>>
>>>> Hi Eugen,
>>>>
>>>> Ok, I'll try to debug that !
>>>
>>> Hi Eugen, while debugging it, I found that there was actually one major
>>> flaw. The clock that I used was the GLCK. If only this clock is
>>> enabled, the TCB can not work, the tcb0_clk must also be enabled for the
>>> TCB to start counting. It worked on my side because I forgot that I had
>>> a quirk in OP-TEE that was enabling the clocks before starting U-Boot.
>>> Without that, it actually could not work.
>>>
>>> Since the tcb0_clk is needed, I switched the driver to use this one
>>> only and it works well on a "bare" setup (ie without OP-TEE). I even
>>> think it could not work with the tcb0_gclk since this one does not
>>> correctly handles the case when the parent clock is the slow_clk (which
>>> might be the case after booting) and in this case, will return 0 as
>>> rate (this is by the way supported in your CCF branch).
>>>
>>> Anyway, I fixed the driver to use the tcb0_clk and thus it should work
>>> much better. I will send a V8 and it should work with U-Boot and the
>>> SPL.
>>>
>>> By the way, do you have any documentation which explains how to use
>>> U-Boot SPL on sama5d2_xplained (or other) board ?
>>
>> Hi Clement,
>>
>> I don't have any documentation at hand, but what I can say in a few
>> words is that once you build the SPL, you should have a file
>> spl/boot.bin and this binary should replace our stage2 at91 bootloader
>> at91bootstrap.
>>
>> Unlike at91bootstrap, the SPL has all the drivers inside and can boot
>> from all supported media, however, SPL will read a dedicated CPU
>> register where the stage1 has written some bits representing the index
>> of the NVM *used to copy the boot.bin from* . The SPL will then attempt
>> to read the u-boot.bin from the same media. (you can alter the file name
>> in defconfig ).
> 
> Ok, I successfully booted a SPL < 64k from SD card by disabling MMC
> support after fixing the TCB driver:
> 
> ;4��K3K{�KOp�C�0�
> U-Boot SPL 2022.04-rc3-00023-gf9da67b41a-dirty (Mar 31 2022 - 09:23:48
> +0200)
> SPL: Unsupported Boot Device!
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
> 
> However, as soon as the size is greater than 64K, it does not boot
> anymore. Looks like you already had the same problem. Do you have any
> more documentation which would allow me to boot a u-boot.bin file from
> this SPL easily ?

Hi,

If size is above 64k, it won't fit into the SRAM, thus the stage1 won't 
copy it at all, and abort booting it.

For arm926 there is a build time check for the size in the lds linker 
script, but it looks like for the armv7 it's missing... No idea why , if 
it was removed, I haven't checked.

I don't have any more documentation.

> 
> By the way, since u-boot,dm-pre-reloc is needed in the device-tree to
> be used with the SPL, I will probably add them to my series
> (property which by the way is missing for the PIT timer
> device-tree node).

Yes, definitely. You can send a prequel or separate patch to add this to 
the PIT timer if you like.
And have this property for your node yes.

Normally all u-boot properties should be in a separate dtsi file, but 
this is an old DT in uboot so I think it's fine to have it in the base DT.

Eugen

> 
> Thanks,
> 
> Clément
> 
> --
> Clément Léger,
> Embedded Linux and Kernel engineer at Bootlin
> https://bootlin.com
>