Re: [PATCH 5/5] arm: kirkwood: Do not overwrite CONFIG_SYS_TCLK

From: "Pali Rohár" <pali@kernel.org>
To: Chris Packham <judge.packham@gmail.com>
Cc: "Stefan Roese" <sr@denx.de>, "Baruch Siach" <baruch@tkos.co.il>,
	"Dirk Eibach" <dirk.eibach@gdsys.cc>,
	u-boot <u-boot@lists.denx.de>, "Marek Behún" <marek.behun@nic.cz>,
	"Dennis Gilmore" <dgilmore@redhat.com>,
	"Mario Six" <mario.six@gdsys.cc>,
	"Jon Nettleton" <jon@solid-run.com>
Subject: Re: [PATCH 5/5] arm: kirkwood: Do not overwrite CONFIG_SYS_TCLK
Date: Sun, 1 Aug 2021 12:25:11 +0200	[thread overview]
Message-ID: <20210801102511.2qtqytqmvofs2vib@pali> (raw)
In-Reply-To: <CAFOYHZBubTeG+gHSM4gRU43CAh_s5mMcYtRKeLOZr8rQCL=dzQ@mail.gmail.com>

On Sunday 01 August 2021 20:07:16 Chris Packham wrote:
> On Sun, Aug 1, 2021 at 12:23 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > Config option CONFIG_SYS_TCLK is set by kw88f6281.h and kw88f6192.h files
> > to correct SOC/platform value. So do not overwrite it in board config
> > include files.
> >
> > Kirkwood 88F6180 and 88F6192 uses 166 MHz TCLK and Kirkwood 88F6281 uses
> > 200 MHz TCLK.
> >
> 
> It's been a while since I worked with kirkwood but I thought that
> there was hardware strapping for the TCLK.

Interesting... Because I took above information from Kirkwood hardware specifications...

88F6180: https://web.archive.org/web/20130730091654/http://www.marvell.com/embedded-processors/kirkwood/assets/HW_88F6180_OpenSource.pdf
88F6192: https://web.archive.org/web/20121021182835/http://www.marvell.com/embedded-processors/kirkwood/assets/HW_88F619x_OpenSource.pdf
88F6281: https://web.archive.org/web/20120620073511/http://www.marvell.com/embedded-processors/kirkwood/assets/HW_88F6281_OpenSource.pdf

And there are specified fixed TCLK values.

> If I understand correctly
> the defines in kw88f6281.h/kw88f6192.h were sensible defaults but
> boards were able to override it to reflect the hardware configuration.

Anyway, I think that this patch should not cause issue as it is changing
only two board config files and removing redefinition of CONFIG_SYS_TCLK
macro which is set to the same value as in kw88f61**.h files.

> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  arch/arm/mach-kirkwood/include/mach/kw88f6281.h | 2 --
> >  include/configs/lacie_kw.h                      | 5 -----
> >  include/configs/lsxl.h                          | 2 --
> >  3 files changed, 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-kirkwood/include/mach/kw88f6281.h b/arch/arm/mach-kirkwood/include/mach/kw88f6281.h
> > index 33e741420781..87406081cf54 100644
> > --- a/arch/arm/mach-kirkwood/include/mach/kw88f6281.h
> > +++ b/arch/arm/mach-kirkwood/include/mach/kw88f6281.h
> > @@ -15,8 +15,6 @@
> >  #define KW_REGS_PHY_BASE               KW88F6281_REGS_PHYS_BASE
> >
> >  /* TCLK Core Clock definition */
> > -#ifndef CONFIG_SYS_TCLK
> >  #define CONFIG_SYS_TCLK        200000000 /* 200MHz */
> > -#endif
> >
> >  #endif /* _ASM_ARCH_KW88F6281_H */
> > diff --git a/include/configs/lacie_kw.h b/include/configs/lacie_kw.h
> > index 420c1d49b08e..88f784f1f0fd 100644
> > --- a/include/configs/lacie_kw.h
> > +++ b/include/configs/lacie_kw.h
> > @@ -39,11 +39,6 @@
> >  #endif
> >  #define CONFIG_SKIP_LOWLEVEL_INIT      /* disable board lowlevel_init */
> >
> > -/*
> > - * Core clock definition
> > - */
> > -#define CONFIG_SYS_TCLK                        166000000 /* 166MHz */
> > -
> >  /*
> >   * SDRAM configuration
> >   */
> > diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
> > index 0c0ab2486e23..a4a4739d0dd7 100644
> > --- a/include/configs/lsxl.h
> > +++ b/include/configs/lsxl.h
> > @@ -13,11 +13,9 @@
> >  #if defined(CONFIG_LSCHLV2)
> >  #define CONFIG_SYS_KWD_CONFIG $(CONFIG_BOARDDIR)/kwbimage-lschl.cfg
> >  #define CONFIG_MACH_TYPE 3006
> > -#define CONFIG_SYS_TCLK 166666667 /* 166 MHz */
> >  #elif defined(CONFIG_LSXHL)
> >  #define CONFIG_SYS_KWD_CONFIG $(CONFIG_BOARDDIR)/kwbimage-lsxhl.cfg
> >  #define CONFIG_MACH_TYPE 2663
> > -/* CONFIG_SYS_TCLK is 200000000 by default */
> >  #else
> >  #error "unknown board"
> >  #endif
> > --
> > 2.20.1
> >