lkml.org 
[lkml]   [2013]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/7] clk: tegra: Add support for PLLSS
On Tue, Oct 15, 2013 at 10:20:03PM +0200, Stephen Warren wrote:
> On 10/15/2013 09:14 AM, Peter De Schrijver wrote:
> > Tegra124 introduces a new PLL type, PLLSS. Add support for it.
>
> > diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
>
>
> > +static int clk_pllss_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
>
> This function seems pretty generic. Is it possible to share a bit more
> code with any of the other pllXXX_set_rate() functions?
>
> > +struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
> > + void __iomem *clk_base, unsigned long flags,
> > + struct tegra_clk_pll_params *pll_params,
> > + spinlock_t *lock)
>
> > + val = pll_readl_base(pll);
> > + if (val & PLLSS_REF_SRC_SEL_MASK) {
> > + WARN(1, "Unknown parent selected for %s: %d\n", name,
> > + (val & PLLSS_REF_SRC_SEL_MASK) >>
> > + PLLSS_REF_SRC_SEL_SHIFT);
> > + kfree(pll);
> > + return ERR_PTR(-EINVAL);
> > + }
>
> If there's a field in HW that muxes the clock input between n clocks,
> why does this function assume there's a single parent for this PLL, by
> taking a "const char *parent_name" parameter?
>
> What happens if the bootloader changed this field in HW; is the kernel
> simply not able to boot?
>

This logic comes from downstream. I guess it means we're running in an
unvalidated configuration. Do you think we should expose all parents
anyway? Even if not all configurations have been validated?
(which is quite likely)

Cheers,

Peter.


\
 
 \ /
  Last update: 2013-10-16 10:41    [W:0.101 / U:2.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site