Re: [PATCH 3/3] clk: tegra: Properly setup PWM clock on Tegra30

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2346 bytes --]

On Tue, Oct 29, 2013 at 01:45:38PM -0600, Stephen Warren wrote:
> On 10/29/2013 09:51 AM, Thierry Reding wrote:
> > The clock for the PWM controller is slightly different from other
> > peripheral clocks on Tegra30. The clock source mux field start at
> > bit position 28 rather than 30.
> 
> I think you need to CC this series to Mike Turquette and relevant lists
> for review. While Peter is sending pull requests to Mike for the Tegra
> clock driver, Mike still needs to see the patches before that happens.

I meant for this to be reviewed primarily by Peter and/or Prashant
before sending it to Mike "officially" because I wasn't at all sure if
it was correct, even though it fixed the issue I was seeing on Cardhu.

But Cc'ing Mike probably wouldn't have hurt either, so I'll just add him
from the start next time.

> > diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> 
> > @@ -836,7 +837,6 @@ static struct tegra_clk tegra30_clks[tegra_clk_max] __initdata = {
> >  	[tegra_clk_extern1] = { .dt_id = TEGRA30_CLK_EXTERN1, .present = true },
> >  	[tegra_clk_extern2] = { .dt_id = TEGRA30_CLK_EXTERN2, .present = true },
> >  	[tegra_clk_extern3] = { .dt_id = TEGRA30_CLK_EXTERN3, .present = true },
> > -	[tegra_clk_pwm] = { .dt_id = TEGRA30_CLK_PWM, .present = true },
> 
> I think you still need an entry in this table; isn't it used by the
> DT->internal clock ID translation function?

As far as I can tell, this is used by the generic code to determine
which of the generic clocks to register. If TEGRA30_CLK_PWM is kept
within this list, tegra_periph_clk_init() will register the clock a
second time.

> Either way, it seems like this patch might want to add a
> tegra_clk_pwm_tegra30 so that the common C files can still implement
> this clock, just with different parameters?

That's pretty much what this patch does, isn't it? It adds a custom
entry for the PWM clock to the Tegra30-specific tegra_periph_clk_list
and keeps the common one from being registered by dropping the entry
from tegra30_clks.

The same is already done on Tegra20, where the PWM clock is similarly
weird. On Tegra114 and Tegra124 the clock is still weird, but it can be
tweaked into behaving more commonly by lying about the actual position
and size of the mux field.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]