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

[Tom Rini <trini@konsulko.com>]

[-- 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 --]