[PATCH 0/3] board: ti: common: setup mux and debounce for 32k RTC crystal

[PATCH 0/3] board: ti: common: setup mux and debounce for 32k RTC crystal

[Bryan Brattlof]

Hello everyone!

The starter kit boards for Texas Instruments' am62xxx extended SoC
family all have the same discrete 32k crystal to provide a more accurate
clock source if needed.

Up until now this has not been needed, however as more features are
starting to rely on the accuracy of the internal RTC we should switch to
using this crystal.

This little series adds a board_rtc_init() function in the
board/ti/common directory to setup this crystal for the am62x and am62ax
starter kits.

Thanks for reviewing!
~Bryan

Bryan Brattlof (3):
  board: ti: common: add rtc setup to common folder
  configs: am62ax: setup the 32k RTC crystal
  configs: am62x: move 32K RTC crystal to common

 board/ti/am62ax/evm.c            |  5 ++++
 board/ti/am62x/evm.c             |  5 ++++
 board/ti/common/Kconfig          |  8 ++++++
 board/ti/common/rtc.c            | 47 ++++++++++++++++++++++++++++++++
 configs/am62ax_evm_a53_defconfig |  1 +
 configs/am62x_evm_a53_defconfig  |  1 +
 6 files changed, 67 insertions(+)
 create mode 100644 board/ti/common/rtc.c


base-commit: e17d174773e9ba9447596708e702b7382e47a6cf
-- 
2.42.0

[PATCH 1/3] board: ti: common: add rtc setup to common folder

[Bryan Brattlof]

All of the starter kit boards for the am62xxx extended family utilize
the same 32k crystal oscillator for a more accurate clock for the RTC
instance. Add the setup the clock mux and debounce configuration to the
common board directory so the entire am62xxx extended family can utilize
it.

Signed-off-by: Bryan Brattlof <bb@ti.com>
---
 board/ti/common/Kconfig |  8 +++++++
 board/ti/common/rtc.c   | 47 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 board/ti/common/rtc.c

diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig
index 49edd98014ab7..56a65c0a402bb 100644
--- a/board/ti/common/Kconfig
+++ b/board/ti/common/Kconfig
@@ -1,3 +1,11 @@
+config BOARD_HAS_32K_RTC_CRYSTAL
+	bool "Enable the 32k crystial for RTC"
+	help
+	   Some of Texas Instrument's Starter-Kit boards have
+	   an onboard 32k crystal. Select this option if you wish Uboot
+	   to enable this crystal for Linux
+	default n
+
 config TI_I2C_BOARD_DETECT
 	bool "Support for Board detection for TI platforms"
 	help
diff --git a/board/ti/common/rtc.c b/board/ti/common/rtc.c
new file mode 100644
index 0000000000000..e117a927765c5
--- /dev/null
+++ b/board/ti/common/rtc.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * RTC setup for TI Platforms
+ *
+ * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
+ */
+#include <asm/arch/hardware.h>
+#include <asm/io.h>
+#include <log.h>
+
+#define WKUP_CTRLMMR_DBOUNCE_CFG1 0x04504084
+#define WKUP_CTRLMMR_DBOUNCE_CFG2 0x04504088
+#define WKUP_CTRLMMR_DBOUNCE_CFG3 0x0450408c
+#define WKUP_CTRLMMR_DBOUNCE_CFG4 0x04504090
+#define WKUP_CTRLMMR_DBOUNCE_CFG5 0x04504094
+#define WKUP_CTRLMMR_DBOUNCE_CFG6 0x04504098
+
+void board_rtc_init(void)
+{
+	u32 val;
+
+	/* We have 32k crystal, so lets enable it */
+	val = readl(MCU_CTRL_LFXOSC_CTRL);
+	val &= ~(MCU_CTRL_LFXOSC_32K_DISABLE_VAL);
+	writel(val, MCU_CTRL_LFXOSC_CTRL);
+
+	/* Add any TRIM needed for the crystal here.. */
+	/* Make sure to mux up to take the SoC 32k from the crystal */
+	writel(MCU_CTRL_DEVICE_CLKOUT_LFOSC_SELECT_VAL,
+	       MCU_CTRL_DEVICE_CLKOUT_32K_CTRL);
+
+	/* Setup debounce conf registers - arbitrary values.
+	 * Times are approx
+	 */
+	/* 1.9ms debounce @ 32k */
+	writel(WKUP_CTRLMMR_DBOUNCE_CFG1, 0x1);
+	/* 5ms debounce @ 32k */
+	writel(WKUP_CTRLMMR_DBOUNCE_CFG2, 0x5);
+	/* 20ms debounce @ 32k */
+	writel(WKUP_CTRLMMR_DBOUNCE_CFG3, 0x14);
+	/* 46ms debounce @ 32k */
+	writel(WKUP_CTRLMMR_DBOUNCE_CFG4, 0x18);
+	/* 100ms debounce @ 32k */
+	writel(WKUP_CTRLMMR_DBOUNCE_CFG5, 0x1c);
+	/* 156ms debounce @ 32k */
+	writel(WKUP_CTRLMMR_DBOUNCE_CFG6, 0x1f);
+}
-- 
2.42.0

[PATCH 2/3] configs: am62ax: setup the 32k RTC crystal

[Bryan Brattlof]

The am62ax utilizes the same 32k crystal for a more accurate RTC clock
source. Enable the configuration to set this up for Linux.

Signed-off-by: Bryan Brattlof <bb@ti.com>
---
 board/ti/am62ax/evm.c            | 5 +++++
 configs/am62ax_evm_a53_defconfig | 1 +
 2 files changed, 6 insertions(+)

diff --git a/board/ti/am62ax/evm.c b/board/ti/am62ax/evm.c
index f2dd3b4192ee0..6e031784766c4 100644
--- a/board/ti/am62ax/evm.c
+++ b/board/ti/am62ax/evm.c
@@ -14,8 +14,13 @@
 #include <fdt_support.h>
 #include <spl.h>
 
+#include "../common/rtc.c"
+
 int board_init(void)
 {
+	if (IS_ENABLED(CONFIG_BOARD_HAS_32K_RTC_CRYSTAL))
+		board_rtc_init();
+
 	return 0;
 }
 
diff --git a/configs/am62ax_evm_a53_defconfig b/configs/am62ax_evm_a53_defconfig
index d0a34c75505d2..e9e969c842d7d 100644
--- a/configs/am62ax_evm_a53_defconfig
+++ b/configs/am62ax_evm_a53_defconfig
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ARCH_K3=y
 CONFIG_SYS_MALLOC_F_LEN=0x8000
+CONFIG_BOARD_HAS_32K_RTC_CRYSTAL=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
-- 
2.42.0

[PATCH 3/3] configs: am62x: move 32K RTC crystal to common

[Bryan Brattlof]

The am62x utilizes the same 32k crystal for a more accurate RTC clock
source. Enable the configuration to set this up for Linux.

Signed-off-by: Bryan Brattlof <bb@ti.com>
---
 board/ti/am62x/evm.c            | 5 +++++
 configs/am62x_evm_a53_defconfig | 1 +
 2 files changed, 6 insertions(+)

diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c
index ad939088402e4..1b2ff7e5c2443 100644
--- a/board/ti/am62x/evm.c
+++ b/board/ti/am62x/evm.c
@@ -19,6 +19,8 @@
 #include <asm/arch/hardware.h>
 #include <dm/uclass.h>
 
+#include "../common/rtc.c"
+
 DECLARE_GLOBAL_DATA_PTR;
 
 #if CONFIG_IS_ENABLED(SPLASH_SCREEN)
@@ -46,6 +48,9 @@ int splash_screen_prepare(void)
 
 int board_init(void)
 {
+	if (IS_ENABLED(CONFIG_BOARD_HAS_32K_RTC_CRYSTAL))
+		board_rtc_init();
+
 	return 0;
 }
 
diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
index df2511546eab7..e9ee265fd7a91 100644
--- a/configs/am62x_evm_a53_defconfig
+++ b/configs/am62x_evm_a53_defconfig
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ARCH_K3=y
 CONFIG_SYS_MALLOC_F_LEN=0x8000
+CONFIG_BOARD_HAS_32K_RTC_CRYSTAL=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
-- 
2.42.0

Re: [PATCH 1/3] board: ti: common: add rtc setup to common folder

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

On Tue, Nov 07, 2023 at 05:21:41PM -0600, Bryan Brattlof wrote:

> All of the starter kit boards for the am62xxx extended family utilize
> the same 32k crystal oscillator for a more accurate clock for the RTC
> instance. Add the setup the clock mux and debounce configuration to the
> common board directory so the entire am62xxx extended family can utilize
> it.
> 
> Signed-off-by: Bryan Brattlof <bb@ti.com>
[snip]
> diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig
> index 49edd98014ab7..56a65c0a402bb 100644
> --- a/board/ti/common/Kconfig
> +++ b/board/ti/common/Kconfig
> @@ -1,3 +1,11 @@
> +config BOARD_HAS_32K_RTC_CRYSTAL
> +	bool "Enable the 32k crystial for RTC"
> +	help
> +	   Some of Texas Instrument's Starter-Kit boards have
> +	   an onboard 32k crystal. Select this option if you wish Uboot
> +	   to enable this crystal for Linux
> +	default n

No "default n" as that is the default. And we (a) need some depends on
for what families this is found on and then (b) how, if at all, does
this match up with the 32k crystal used on other TI reference platforms
over the years? If this is specific to the K3 families of reference
platforms, the help needs re-phrasing and the filename is too generic.
It's also not a "RTC" in terms of something we can talk to via
drivers/rtc/rtc-uclass.c and drivers/rtc/davinci.c, or in this case
porting the kernel's drivers/rtc/rtc-ti-k3.c over, yes?

Oh, and "U-Boot" not "Uboot". Should see if the checkpatch typo list
can be easily expanded by us, one of these days.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/3] configs: am62ax: setup the 32k RTC crystal

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Tue, Nov 07, 2023 at 05:21:42PM -0600, Bryan Brattlof wrote:

> The am62ax utilizes the same 32k crystal for a more accurate RTC clock
> source. Enable the configuration to set this up for Linux.
> 
> Signed-off-by: Bryan Brattlof <bb@ti.com>
> ---
>  board/ti/am62ax/evm.c            | 5 +++++
>  configs/am62ax_evm_a53_defconfig | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/board/ti/am62ax/evm.c b/board/ti/am62ax/evm.c
> index f2dd3b4192ee0..6e031784766c4 100644
> --- a/board/ti/am62ax/evm.c
> +++ b/board/ti/am62ax/evm.c
> @@ -14,8 +14,13 @@
>  #include <fdt_support.h>
>  #include <spl.h>
>  
> +#include "../common/rtc.c"

Oh goodness, no, we don't include a C file without very good reason.
Build that object based on whatever the right symbol name is (based on
my feedback in 1/3) and find some appropriate header for the function
prototype to be in as well. And this applies to 3/3 as well.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/3] board: ti: common: add rtc setup to common folder

[Vignesh Raghavendra]

On 08/11/23 04:51, Bryan Brattlof wrote:
> All of the starter kit boards for the am62xxx extended family utilize
> the same 32k crystal oscillator for a more accurate clock for the RTC
> instance. Add the setup the clock mux and debounce configuration to the
> common board directory so the entire am62xxx extended family can utilize
> it.
> 
> Signed-off-by: Bryan Brattlof <bb@ti.com>
> ---
>  board/ti/common/Kconfig |  8 +++++++
>  board/ti/common/rtc.c   | 47 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>  create mode 100644 board/ti/common/rtc.c
> 
> diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig
> index 49edd98014ab7..56a65c0a402bb 100644
> --- a/board/ti/common/Kconfig
> +++ b/board/ti/common/Kconfig
> @@ -1,3 +1,11 @@
> +config BOARD_HAS_32K_RTC_CRYSTAL
> +	bool "Enable the 32k crystial for RTC"
> +	help
> +	   Some of Texas Instrument's Starter-Kit boards have
> +	   an onboard 32k crystal. Select this option if you wish Uboot
> +	   to enable this crystal for Linux
> +	default n
> +
>  config TI_I2C_BOARD_DETECT
>  	bool "Support for Board detection for TI platforms"
>  	help
> diff --git a/board/ti/common/rtc.c b/board/ti/common/rtc.c
> new file mode 100644
> index 0000000000000..e117a927765c5
> --- /dev/null
> +++ b/board/ti/common/rtc.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * RTC setup for TI Platforms
> + *
> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +#include <asm/arch/hardware.h>
> +#include <asm/io.h>
> +#include <log.h>
> +
> +#define WKUP_CTRLMMR_DBOUNCE_CFG1 0x04504084
> +#define WKUP_CTRLMMR_DBOUNCE_CFG2 0x04504088
> +#define WKUP_CTRLMMR_DBOUNCE_CFG3 0x0450408c
> +#define WKUP_CTRLMMR_DBOUNCE_CFG4 0x04504090
> +#define WKUP_CTRLMMR_DBOUNCE_CFG5 0x04504094
> +#define WKUP_CTRLMMR_DBOUNCE_CFG6 0x04504098
> +
> +void board_rtc_init(void)
> +{
> +	u32 val;
> +
> +	/* We have 32k crystal, so lets enable it */
> +	val = readl(MCU_CTRL_LFXOSC_CTRL);
> +	val &= ~(MCU_CTRL_LFXOSC_32K_DISABLE_VAL);
> +	writel(val, MCU_CTRL_LFXOSC_CTRL);
> +
> +	/* Add any TRIM needed for the crystal here.. */
> +	/* Make sure to mux up to take the SoC 32k from the crystal */
> +	writel(MCU_CTRL_DEVICE_CLKOUT_LFOSC_SELECT_VAL,
> +	       MCU_CTRL_DEVICE_CLKOUT_32K_CTRL);
> +
> +	/* Setup debounce conf registers - arbitrary values.
> +	 * Times are approx
> +	 */
> +	/* 1.9ms debounce @ 32k */
> +	writel(WKUP_CTRLMMR_DBOUNCE_CFG1, 0x1);
> +	/* 5ms debounce @ 32k */
> +	writel(WKUP_CTRLMMR_DBOUNCE_CFG2, 0x5);
> +	/* 20ms debounce @ 32k */
> +	writel(WKUP_CTRLMMR_DBOUNCE_CFG3, 0x14);
> +	/* 46ms debounce @ 32k */
> +	writel(WKUP_CTRLMMR_DBOUNCE_CFG4, 0x18);
> +	/* 100ms debounce @ 32k */
> +	writel(WKUP_CTRLMMR_DBOUNCE_CFG5, 0x1c);
> +	/* 156ms debounce @ 32k */
> +	writel(WKUP_CTRLMMR_DBOUNCE_CFG6, 0x1f);
> +}


Pad Debounce settings has nothing to do with RTC. This doesn't belong to
board_rtc_init()

-- 
Regards
Vignesh

Re: [PATCH 1/3] board: ti: common: add rtc setup to common folder

[Bryan Brattlof]

On November  8, 2023 thus sayeth Vignesh Raghavendra:
> On 08/11/23 04:51, Bryan Brattlof wrote:
> > All of the starter kit boards for the am62xxx extended family utilize
> > the same 32k crystal oscillator for a more accurate clock for the RTC
> > instance. Add the setup the clock mux and debounce configuration to the
> > common board directory so the entire am62xxx extended family can utilize
> > it.
> > 
> > Signed-off-by: Bryan Brattlof <bb@ti.com>
> > ---

...

> > +	/* Setup debounce conf registers - arbitrary values.
> > +	 * Times are approx
> > +	 */
> > +	/* 1.9ms debounce @ 32k */
> > +	writel(WKUP_CTRLMMR_DBOUNCE_CFG1, 0x1);
> > +	/* 5ms debounce @ 32k */
> > +	writel(WKUP_CTRLMMR_DBOUNCE_CFG2, 0x5);
> > +	/* 20ms debounce @ 32k */
> > +	writel(WKUP_CTRLMMR_DBOUNCE_CFG3, 0x14);
> > +	/* 46ms debounce @ 32k */
> > +	writel(WKUP_CTRLMMR_DBOUNCE_CFG4, 0x18);
> > +	/* 100ms debounce @ 32k */
> > +	writel(WKUP_CTRLMMR_DBOUNCE_CFG5, 0x1c);
> > +	/* 156ms debounce @ 32k */
> > +	writel(WKUP_CTRLMMR_DBOUNCE_CFG6, 0x1f);
> > +}
> 
> 
> Pad Debounce settings has nothing to do with RTC. This doesn't belong to
> board_rtc_init()
> 

Oops.. I have no idea what I was thinking yesterday. This was a really 
half-baked plan all the way through :) 

I'll get this properly sorted out for v2

Thanks for reviewing though
~Bryan

Re: [PATCH 1/3] board: ti: common: add rtc setup to common folder

[Bryan Brattlof]

On November  7, 2023 thus sayeth Tom Rini:
> On Tue, Nov 07, 2023 at 05:21:41PM -0600, Bryan Brattlof wrote:
> 
> > All of the starter kit boards for the am62xxx extended family utilize
> > the same 32k crystal oscillator for a more accurate clock for the RTC
> > instance. Add the setup the clock mux and debounce configuration to the
> > common board directory so the entire am62xxx extended family can utilize
> > it.
> > 
> > Signed-off-by: Bryan Brattlof <bb@ti.com>
> [snip]
> > diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig
> > index 49edd98014ab7..56a65c0a402bb 100644
> > --- a/board/ti/common/Kconfig
> > +++ b/board/ti/common/Kconfig
> > @@ -1,3 +1,11 @@
> > +config BOARD_HAS_32K_RTC_CRYSTAL
> > +	bool "Enable the 32k crystial for RTC"
> > +	help
> > +	   Some of Texas Instrument's Starter-Kit boards have
> > +	   an onboard 32k crystal. Select this option if you wish Uboot
> > +	   to enable this crystal for Linux
> > +	default n
> 
> No "default n" as that is the default. And we (a) need some depends on
> for what families this is found on and then (b) how, if at all, does
> this match up with the 32k crystal used on other TI reference platforms
> over the years? If this is specific to the K3 families of reference
> platforms, the help needs re-phrasing and the filename is too generic.
> It's also not a "RTC" in terms of something we can talk to via
> drivers/rtc/rtc-uclass.c and drivers/rtc/davinci.c, or in this case
> porting the kernel's drivers/rtc/rtc-ti-k3.c over, yes?
> 
> Oh, and "U-Boot" not "Uboot". Should see if the checkpatch typo list
> can be easily expanded by us, one of these days.

Yeah I'll work on the wording.

We just need to toggle a few bits for Linux to get the crystal muxed 
properly for the RTC driver in Linux.

We also toggle a few bits for the debouce settings that apparently I 
didn't separate out from this series.

Thanks for the quick review though.
~Bryan

Re: [PATCH 1/3] board: ti: common: add rtc setup to common folder

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]

On Wed, Nov 08, 2023 at 09:46:34AM -0600, Bryan Brattlof wrote:
> On November  7, 2023 thus sayeth Tom Rini:
> > On Tue, Nov 07, 2023 at 05:21:41PM -0600, Bryan Brattlof wrote:
> > 
> > > All of the starter kit boards for the am62xxx extended family utilize
> > > the same 32k crystal oscillator for a more accurate clock for the RTC
> > > instance. Add the setup the clock mux and debounce configuration to the
> > > common board directory so the entire am62xxx extended family can utilize
> > > it.
> > > 
> > > Signed-off-by: Bryan Brattlof <bb@ti.com>
> > [snip]
> > > diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig
> > > index 49edd98014ab7..56a65c0a402bb 100644
> > > --- a/board/ti/common/Kconfig
> > > +++ b/board/ti/common/Kconfig
> > > @@ -1,3 +1,11 @@
> > > +config BOARD_HAS_32K_RTC_CRYSTAL
> > > +	bool "Enable the 32k crystial for RTC"
> > > +	help
> > > +	   Some of Texas Instrument's Starter-Kit boards have
> > > +	   an onboard 32k crystal. Select this option if you wish Uboot
> > > +	   to enable this crystal for Linux
> > > +	default n
> > 
> > No "default n" as that is the default. And we (a) need some depends on
> > for what families this is found on and then (b) how, if at all, does
> > this match up with the 32k crystal used on other TI reference platforms
> > over the years? If this is specific to the K3 families of reference
> > platforms, the help needs re-phrasing and the filename is too generic.
> > It's also not a "RTC" in terms of something we can talk to via
> > drivers/rtc/rtc-uclass.c and drivers/rtc/davinci.c, or in this case
> > porting the kernel's drivers/rtc/rtc-ti-k3.c over, yes?
> > 
> > Oh, and "U-Boot" not "Uboot". Should see if the checkpatch typo list
> > can be easily expanded by us, one of these days.
> 
> Yeah I'll work on the wording.
> 
> We just need to toggle a few bits for Linux to get the crystal muxed 
> properly for the RTC driver in Linux.

This sounds a whole lot like what AM33XX_ENABLE_RTC32K_OSC symbol (which
could be badly named, too!) is about, and hence some of my other
questions. So please keep in mind the am33xx case here, and also how
likely or not every custom K3 platform is also going to be doing this,
or not. On am33xx, it does look like a few do disable it, but it's in
common code since most custom platforms want it.

> We also toggle a few bits for the debouce settings that apparently I 
> didn't separate out from this series.

Please keep in mind if these too are essentially generic changes for the
SoC family as well (or just how board specific they might be and how
wrong they could be before a problem is seen, ie technical vs
practical considerations).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]