[U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Chin Liang See]

On Sel, 2016-10-18 at 06:00 +0200, Marek Vasut wrote:
> On 10/18/2016 05:22 AM, Chin Liang See wrote:
> > 
> > On Sen, 2016-10-17 at 18:14 +0200, Marek Vasut wrote:
> > > 
> > > On 10/17/2016 05:59 PM, Chin Liang See wrote:
> > > > 
> > > > 
> > > > On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 10/17/2016 05:28 PM, Chin Liang See wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 10/17/2016 05:07 PM, Chin Liang See wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 10/17/2016 03:32 PM, See, Chin Liang wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut
> > > > > > > > > > wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > On 10/13/2016 10:33 AM, Chin Liang See wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > Separate the Clock Manager to support both GEN5
> > > > > > > > > > > > SoC
> > > > > > > > > > > > and
> > > > > > > > > > > > Stratix 10 SoC.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Chin Liang See <clsee at altera.com
> > > > > > > > > > > > >
> > > > > > > > > > > > Cc: Marek Vasut <marex at denx.de>
> > > > > > > > > > > > Cc: Dinh Nguyen <dinguyen at opensource.altera.com
> > > > > > > > > > > > >
> > > > > > > > > > > > Cc: Ley Foon Tan <lftan at altera.com>
> > > > > > > > > > > > Cc: Tien Fong Chee <tfchee at altera.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >  arch/arm/mach-socfpga/clock_manager.c | 8
> > > > > > > > > > > > ++++++++
> > > > > > > > > > > >  1 file changed, 8 insertions(+)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/arch/arm/mach-
> > > > > > > > > > > > socfpga/clock_manager.c
> > > > > > > > > > > > b/arch/arm/mach-
> > > > > > > > > > > > socfpga/clock_manager.c
> > > > > > > > > > > > index aa71636..0d67b3c 100644
> > > > > > > > > > > > --- a/arch/arm/mach-socfpga/clock_manager.c
> > > > > > > > > > > > +++ b/arch/arm/mach-socfpga/clock_manager.c
> > > > > > > > > > > > @@ -10,6 +10,7 @@
> > > > > > > > > > > > 
> > > > > > > > > > > >  DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > > > > 
> > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > > > > > > > > > > >  static const struct socfpga_clock_manager
> > > > > > > > > > > > *clock_manager_base
> > > > > > > > > > > > =
> > > > > > > > > > > >       (struct socfpga_clock_manager
> > > > > > > > > > > > *)SOCFPGA_CLKMGR_ADDRESS;
> > > > > > > > > > > > 
> > > > > > > > > > > > @@ -446,9 +447,11 @@ unsigned int
> > > > > > > > > > > > cm_get_l4_sp_clk_hz(void)
> > > > > > > > > > > > 
> > > > > > > > > > > >       return clock;
> > > > > > > > > > > >  }
> > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */
> > > > > > > > > > > > 
> > > > > > > > > > > >  unsigned int
> > > > > > > > > > > > cm_get_mmc_controller_clk_hz(void)
> > > > > > > > > > > >  {
> > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > > > > > > > > > > >       uint32_t reg, clock = 0;
> > > > > > > > > > > > 
> > > > > > > > > > > >       /* identify the source of MMC clock */
> > > > > > > > > > > > @@ -475,8 +478,12 @@ unsigned int
> > > > > > > > > > > > cm_get_mmc_controller_clk_hz(void)
> > > > > > > > > > > >       /* further divide by 4 as we have fixed
> > > > > > > > > > > > divider
> > > > > > > > > > > > at
> > > > > > > > > > > > wrapper */
> > > > > > > > > > > >       clock /= 4;
> > > > > > > > > > > >       return clock;
> > > > > > > > > > > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
> > > > > > > > > > > > +     return 25000000;
> > > > > > > > > > > Is this always gonna be the case or is this S10VP
> > > > > > > > > > > specific ?
> > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */
> > > > > > > > > > > >  }
> > > > > > > > > > > > 
> > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > > > > > > > > > > >  unsigned int
> > > > > > > > > > > > cm_get_qspi_controller_clk_hz(void)
> > > > > > > > > > > >  {
> > > > > > > > > > > >       uint32_t reg, clock = 0;
> > > > > > > > > > > > @@ -556,3 +563,4 @@ U_BOOT_CMD(
> > > > > > > > > > > >       "display clocks",
> > > > > > > > > > > >       ""
> > > > > > > > > > > Why does the clock display not work on S10 ? Are
> > > > > > > > > > > some
> > > > > > > > > > > functions
> > > > > > > > > > > missing?
> > > > > > > > > > Not for SOCVP. But will be added in later stage
> > > > > > > > > > when
> > > > > > > > > > testing
> > > > > > > > > > against
> > > > > > > > > > emulation
> > > > > > > > > How hard would it be to add this missing
> > > > > > > > > functionality
> > > > > > > > > now ?
> > > > > > > > > 
> > > > > > > > That will take weeks as that need to be validated as
> > > > > > > > whole
> > > > > > > > in
> > > > > > > > emulation
> > > > > > > > platform.
> > > > > > > You mean printing a few clock information based on some
> > > > > > > values
> > > > > > > from
> > > > > > > registers would take weeks ? Why ?
> > > > > > > 
> > > > > > Oh actually I am referring all the managers code such as
> > > > > > full
> > > > > > Clock
> > > > > > Manager, Reset Manager ... plus testing. Testing is the
> > > > > > part
> > > > > > take
> > > > > > some
> > > > > > significant time especially slow when come to emulation.
> > > > > Just use empty functions for the clock init code (since it's
> > > > > not
> > > > > needed
> > > > > on the socvp) and populate the clock reporting functions.
> > > > > That
> > > > > should
> > > > > be
> > > > > simple, right ?
> > > > Can be done but the value won't be meaningful as the register
> > > > is
> > > > uninitialzied. Unless we hardcode to a hard value which might
> > > > not
> > > > sound
> > > > right.
> > > Ha, I see. Is there some sane default for the SoCVP ?
> > > 
> > It will not meaningful as performance is same even with frequency
> > change :) But definitely will address this in coming weeks as I am
> > already working on the emulation. 
> I understand that, but I believe it is still better to provide some
> sensible value rather than just ifdef the whole code out.

I can hardcode some sensible value for that if good.

Thanks
Chin Liang

>