[U-Boot] [PATCH] armv8/fsl-layerscape: fdt: remove SYSCLK frequency fixup for ls1012a

[Scott Wood]

On Fri, 2017-01-20 at 21:38 +0000, york sun wrote:
> On 01/20/2017 01:36 PM, Scott Wood wrote:
> > 
> > On Fri, 2017-01-20 at 16:28 +0000, york sun wrote:
> > > 
> > > On 01/19/2017 07:34 PM, Yangbo Lu wrote:
> > > > 
> > > > 
> > > > Generally SYSCLK frequency is dependent on on-board switch settings.
> > > > It may vary as per requirement, but this doesn't apply to ls1012a.
> > > > ls1012a has its SYSCLK frequencies specified in the RM. The fixup
> > > > for all 'fixed-clock' compatibles of ls1012a would cause incorrect
> > > > SYSCLK frequency values. So remove the SYSCLK frequency fixup for
> > > > ls1012a.
> > > > 
> > > > Fixes: 6f14e25 ("armv8: fsl-lsch3: fixup SYSCLK frequency in device
> > > > tree")
> > > > Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
> > > > ---
> > > >  arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > > > b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > > > index c10ccf9..e59c232 100644
> > > > --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > > > @@ -161,8 +161,10 @@ void ft_cpu_setup(void *blob, bd_t *bd)
> > > >  			       "clock-frequency",
> > > > CONFIG_SYS_NS16550_CLK,
> > > > 1);
> > > >  #endif
> > > > 
> > > > +#ifndef CONFIG_ARCH_LS1012A
> > > >  	do_fixup_by_compat_u32(blob, "fixed-clock",
> > > >  			       "clock-frequency",
> > > > CONFIG_SYS_CLK_FREQ,
> > > > 1);
> > > > +#endif
> > > > 
> > > >  #ifdef CONFIG_PCI
> > > >  	ft_pci_setup(blob, bd);
> > > > 
> > > Yangbo,
> > > 
> > > Why fixing up this clock causes incorect frequency value? The macro
> > > CONFIG_SYS_CLK_FREQ is defined as 125MHz for ls1012a.
> > Because ls1012a has two different input frequencies -- 125 MHz for the
> > platform PLL and 100 MHz for the core PLLs.  When we added a second fixed-
> > clock node for the latter, U-Boot was overwriting it.
> > 
> > While the ifdef solves this immediate problem, it doesn't fix the
> > underlying
> > problem that this fixup is overly broad.  It should identify the specific
> > node
> > it's looking for, and not overwrite every fixed-clock node it finds.
> > 
> So current code tries to fix up any node with "fixed-clock"? That's not 
> good. What if we have multiple fixed clocks?
> 

That is exactly the problem.  This patch avoids the issue on ls1012a but not
in general.

-Scott