Re: [RFC 6/9] clk: ti: add support for omap4 module clocks

[Michael Turquette]

Quoting Tero Kristo (2016-01-04 11:15:36)
> On 01/04/2016 06:37 PM, Tony Lindgren wrote:
> > * Russell King - ARM Linux  [160104 06:43]:
> >> On Mon, Jan 04, 2016 at 03:27:57PM +0200, Tero Kristo wrote:
> >>> On 01/04/2016 12:21 PM, Geert Uytterhoeven wrote:
>  FWIW, there are small loops with just a cpu_relax() in various clock 
>  drivers
>  under drivers/clk/shmobile/.
> >>>
> >>> Just did a quick profiling round, and the clk_enable/disable delay loops
> >>> take anything from 0...1500ns, most typically consuming some 400-600ns. 
> >>> So,
> >>> based on this, dropping the udelay and adding cpu_relax instead looks 
> >>> like a
> >>> good change. I just verified that changing the udelay to cpu_relax works
> >>> fine also, I just need to change the bail-out period to be something sane.
> >>
> >> Was that profiling done with lockdep/lock debugging enabled or disabled?
> 
> omap2plus_defconfig, so lockdep was enabled. The profiling was done 
> around the while {} block though, which should not have any locks within 
> it (except for the SCM clocks, which may explain some of the higher 
> latency numbers seen.)
> 
> > And also the thing to check from the hw folks is what all do these clkctrl
> > bits really control. If they group together the OCP clock and an extra
> > functional clock for some devices the delays could be larger.
> 
> Does it matter really? The latencies are only imposed to the device in 
> question, and lets face it, the same latencies are there already with 
> the hwmod implementation. This series moves the implementation under 
> clock driver with as less modifications as possible to avoid any problems.

So long as we can all convince ourselves that the flaw is not a flaw
then I'm OK with it. No bugs were ever introduced that way ;-)

But in fairness, we've had these delays in the .enable callbacks for a
while, so this patch does not introduce the regression. Furthermore it
does clean up some code that needs more work, and I don't want to delay
that.

I won't NACK the patch due to the delays, but it would be nice to
revisit it some day.

Regards,
Mike

> 
> > In general, I think we need to get rid of pm_runtime_irq_safe usage to
> > allow clocks to sleep properly. The other option is to allow toggling
> > pm_runtime_irq_safe but that probably gets super messy.
> 
> That is something not to be done with this set though.
> 
> -Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 6/9] clk: ti: add support for omap4 module clocks

[Michael Turquette]

Quoting Tero Kristo (2016-01-03 23:36:05)
> On 01/01/2016 07:48 AM, Michael Turquette wrote:
> > Hi Tero,
> >
> > Quoting Tero Kristo (2015-12-18 05:58:58)
> >> Previously, hwmod core has been used for controlling the hwmod level
> >> clocks. This has certain drawbacks, like being unable to share the
> >> clocks for multiple users, missing usecounting and generally being
> >> totally incompatible with common clock framework.
> >>
> >> Add support for new clock type under the TI clock driver, which will
> >> be used to convert all the existing hwmdo clocks to. This helps to
> >> get rid of the clock related hwmod data from kernel and instead
> >> parsing this from DT.
> >
> > I'm really happy to see this series. Looks pretty good to me.
> >
> >> +static int _omap4_hwmod_clk_enable(struct clk_hw *hw)
> >> +{
> >> +   struct clk_hw_omap *clk = to_clk_hw_omap(hw);
> >> +   u32 val;
> >> +   int timeout = 0;
> >> +   int ret;
> >> +
> >> +   if (!clk->enable_bit)
> >> +   return 0;
> >> +
> >> +   if (clk->clkdm) {
> >> +   ret = ti_clk_ll_ops->clkdm_clk_enable(clk->clkdm, hw->clk);
> >> +   if (ret) {
> >> +   WARN(1,
> >> +"%s: could not enable %s's clockdomain %s: 
> >> %d\n",
> >> +__func__, clk_hw_get_name(hw),
> >> +clk->clkdm_name, ret);
> >> +   return ret;
> >> +   }
> >> +   }
> >> +
> >> +   val = ti_clk_ll_ops->clk_readl(clk->enable_reg);
> >> +
> >> +   val &= ~OMAP4_MODULEMODE_MASK;
> >> +   val |= clk->enable_bit;
> >> +
> >> +   ti_clk_ll_ops->clk_writel(val, clk->enable_reg);
> >> +
> >> +   /* Wait until module is enabled */
> >> +   while (!_omap4_is_ready(val)) {
> >> +   udelay(1);
> >
> > This should really be a .prepare callback if you plan to keep the delays
> > in there.
> 
> If this is changed to a .prepare, then all OMAP power management is 
> effectively ruined as all clocks are going to be enabled all the time. 

Let's not ruin system PM.

> hwmod core doesn't support .prepare/.enable at the moment that well, and 
> changing that is going to be a big burden (educated guess, haven't 
> checked this yet)... The call chain that comes here is:
> 
> device driver -> pm_runtime -> hwmod_core -> hwmod_clk_enable / disable.

Right, and for calls to pm_runtime_get/put from process context it
should result in a call to clk_prepare_enable/clk_disable_unprepare. I
guess that change is hugely invasive from your statements above?

> 
> The delay within this function should usually be pretty short, just to 
> wait that the module comes up from idle.
> 
> I recall the discussions regarding the udelays within clk_enable/disable 
> calls, but what is the preferred approach then?

There are many cases where a clk only provides .{un}prepare ops and does
NOT provide any .{en,dis}able ops.

> Typically 
> clk_enable/disable just becomes a NOP

Yes, it becomes a NOP (though it is critical that drivers with knowledge
of this do not try to skip the step of calling clk_enable).

> if it is not allowed to wait for 
> hardware to complete transitioning before exiting the function.

The clk.h api clearly states that clk_prepare must be called and
complete before calling clk_enable. So if a clk only provides a .prepare
with delays but no .enable, and a consumer driver complies with that api
rule then we're guaranteed to have a toggling line when clk_enable
returns.

Regards,
Mike

> 
> -Tero
> 
> >
> > Regards,
> > Mike
> >
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 6/9] clk: ti: add support for omap4 module clocks

[Michael Turquette]

Hi Tero,

Quoting Tero Kristo (2015-12-18 05:58:58)
> Previously, hwmod core has been used for controlling the hwmod level
> clocks. This has certain drawbacks, like being unable to share the
> clocks for multiple users, missing usecounting and generally being
> totally incompatible with common clock framework.
> 
> Add support for new clock type under the TI clock driver, which will
> be used to convert all the existing hwmdo clocks to. This helps to
> get rid of the clock related hwmod data from kernel and instead
> parsing this from DT.

I'm really happy to see this series. Looks pretty good to me.

> +static int _omap4_hwmod_clk_enable(struct clk_hw *hw)
> +{
> +   struct clk_hw_omap *clk = to_clk_hw_omap(hw);
> +   u32 val;
> +   int timeout = 0;
> +   int ret;
> +
> +   if (!clk->enable_bit)
> +   return 0;
> +
> +   if (clk->clkdm) {
> +   ret = ti_clk_ll_ops->clkdm_clk_enable(clk->clkdm, hw->clk);
> +   if (ret) {
> +   WARN(1,
> +"%s: could not enable %s's clockdomain %s: %d\n",
> +__func__, clk_hw_get_name(hw),
> +clk->clkdm_name, ret);
> +   return ret;
> +   }
> +   }
> +
> +   val = ti_clk_ll_ops->clk_readl(clk->enable_reg);
> +
> +   val &= ~OMAP4_MODULEMODE_MASK;
> +   val |= clk->enable_bit;
> +
> +   ti_clk_ll_ops->clk_writel(val, clk->enable_reg);
> +
> +   /* Wait until module is enabled */
> +   while (!_omap4_is_ready(val)) {
> +   udelay(1);

This should really be a .prepare callback if you plan to keep the delays
in there.

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] clk: ti: omap5+: dpll: implement errata i810

[Michael Turquette]

Hi Tero,

On 12/16, Tony Lindgren wrote:
> * Tero Kristo <t-kri...@ti.com> [151216 01:00]:
> > Errata i810 states that DPLL controller can get stuck while transitioning
> > to a power saving state, while its M/N ratio is being re-programmed.
> > 
> > As a workaround, before re-programming the M/N ratio, SW has to ensure
> > the DPLL cannot start an idle state transition. SW can disable DPLL
> > idling by setting the DPLL AUTO_DPLL_MODE=0 or keeping a clock request
> > active by setting a dependent clock domain in SW_WKUP.
> > 
> > This errata is known to impact OMAP5 and DRA7 chips, but lets enable it
> > unconditionally to avoid any potential problems with earlier generation
> > SoCs also.
> > 
> > Signed-off-by: Tero Kristo <t-kri...@ti.com>
> > ---
> > v2: made the fix to be applied unconditionally on all OMAP3+ SoCs
> 
> Thanks looks good to me now:
> 
> Acked-by: Tony Lindgren <t...@atomide.com>

Patch looks good to me too.

Stephen and I were discussing clk pull requests, more specifically how
to vet the contents of them. So we came up with the idea to add our acks
to patches that we take into our tree, but that we expect to get batched
up into a pull request. To that end:

Acked-by: Michael Turquette <mturque...@baylibre.com>

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] clk: ti: Add support for dm814x ADPLL

[Michael Turquette]

On 12/22, Tony Lindgren wrote:
> * Tero Kristo <t-kri...@ti.com> [151222 12:28]:
> > On 12/22/2015 05:27 PM, Tony Lindgren wrote:
> > >On dm814x we have 13 ADPLLs with 3 to 4 outputs on each. The
> > >ADPLLs have several dividers and muxes controlled by a shared
> > >control register for each PLL.
> > >
> > >Note that for the clocks to work as device drivers for booting on
> > >dm814x, this patch depends on "ARM: OMAP2+: Change core_initcall
> > >levels to postcore_initcall".
> > >
> > >Also note that this patch does not implement clk_set_rate for the
> > >PLL, that will be posted later on when available.
> > >
> > >Cc: Michael Turquette <mturque...@baylibre.com>
> > >Cc: Stephen Boyd <sb...@codeaurora.org>
> > >Cc: Tero Kristo <t-kri...@ti.com>
> > >Signed-off-by: Tony Lindgren <t...@atomide.com>
> > >---
> > >
> > >If no more comments, Tero can you please apply into an immutable
> > >branch against v4.4-rc1 that I can merge in too?
> > >
> > >Changes since v3:
> > >
> > >- We want to create the clkdev entry for all clocks, not just outputs
> > >- ti_adpll_wait_lock loops did not do the right thing
> > >- We want to use CLK_GET_RATE_NOCACHE in ti_adpll_init_dco
> > 
> > I have just one comment below still, once that is addressed:
> > 
> > Conditionally-acked-by: Tero Kristo <t-kri...@ti.com>
> > 
> > Stephen / Michael, can you pick this up for next merge? I don't have
> > anything else coming for the window this time, and I am probably going to be
> > on vacation just nicely to not be able to push anything anyway.
> 
> Also, I managed to remove the dependency to the dts changes. So there's
> no longer any need to set up an immutable branch for this patch.

Can you split the binding description into a separate patch and send it
to the dt mailing list? Feel free to add my Ack to it.

Stephen and I are trying to not take that stuff anymore.

Regards,
Mike

> 
> > 
> > 
> > >+
> > >+/* Warn if clkout or clkoutx2 try to set unavailable parent */
> > >+static int ti_adpll_clkout_set_parent(struct clk_hw *hw, u8 index)
> > >+{
> > >+  struct ti_adpll_clkout_data *co = to_clkout(hw);
> > >+  struct ti_adpll_data *d = co->adpll;
> > >+
> > >+  if (ti_adpll_clock_is_bypass(d) != index)
> > >+  return -EAGAIN;
> > >+
> > 
> > I think this part is still somewhat weird. You are not doing anything useful
> > in this function, so do you need to implement it at all? Just returning
> > -EINVAL always might work also. EAGAIN is wrong return value anyway as it
> > can pretty much never succeed.
> 
> OK thanks sounds good to me, I'll check it this after lunch.
> 
> Also noticed the do_div should be div64_u64 so v5 coming later
> today.
> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC RFT 2/3] clk: clk_put WARNs if user has not disabled clk

[Michael Turquette]

Quoting Russell King - ARM Linux (2015-10-21 03:59:32)
> On Wed, Oct 21, 2015 at 11:50:07AM +0200, Geert Uytterhoeven wrote:
> > Hi Mike, Russell,
> > 
> > On Tue, Oct 20, 2015 at 2:40 PM, Michael Turquette
> > <mturque...@baylibre.com> wrote:
> > > Why not keep the reference to the struct clk after get'ing it the first
> > > time?
> > 
> > And store it where?
> 
> Not my problem :)
> 
> Users are supposed to hold on to the reference obtained via clk_get()
> while they're making use of the clock: in some implementations, this
> increments the module use count if the clock driver is a module, and
> may have other effects too.
> 
> Dropping that while you're still requiring the clock to be enabled is
> unsafe: if it is provided by a module, then removing and reinserting
> the module may very well change the enabled state of the clock, it
> most certainly will disrupt the enable count.
> 
> It's always been this way, right from the outset, and when I've seen
> people doing this bollocks, I've always pointed out that it's wrong.
> Generally, people will fix it once they become aware of it, so it's
> really that people just don't like reading and conforming to published
> API requirements.
> 
> I think the root cause is that people just don't like reading what
> other people write in terms of documentation, and they prefer to go
> off and do their own thing, provided it works for them.

Right, so in other words this problem must be solved by the caller of
clk_get, as it should be. I have never much looked at the pm clk code in
question, but I gave it a quick look and came up with some example code
that does not compile, in an effort to be as helpful as possible.

Basically I added a flex array to struct pm_clk_notifier_block, so that
now there are two flex arrays which is stupid. But I am too lazy to
replace the nbclk->clks thing with a malloc after walking all of the
clk_id's to figure out the number of clocks. Or we could just add
.num_clk to the struct, fix up all 4 users of it and drop the NULL
sentinel used the .clk_id's... Hmm that would have been better.

Anyways here is the ugly, non-compiling code. I'll take another look at
it in one year if no one else beats me to it.

Regards,
Mike



diff --git a/arch/arm/mach-davinci/pm_domain.c 
b/arch/arm/mach-davinci/pm_domain.c
index 78eac2c..b46e5ce 100644
--- a/arch/arm/mach-davinci/pm_domain.c
+++ b/arch/arm/mach-davinci/pm_domain.c
@@ -24,6 +24,7 @@ static struct dev_pm_domain davinci_pm_domain = {
 static struct pm_clk_notifier_block platform_bus_notifier = {
.pm_domain = _pm_domain,
.con_ids = { "fck", "master", "slave", NULL },
+   .clks = { ERR_PTR(-EAGAIN), ERR_PTR(-EAGAIN), ERR_PTR(-EAGAIN) },
 };
 
 static int __init davinci_pm_runtime_init(void)
diff --git a/arch/arm/mach-keystone/pm_domain.c 
b/arch/arm/mach-keystone/pm_domain.c
index e283939..d21c18b 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -27,6 +27,7 @@ static struct dev_pm_domain keystone_pm_domain = {
 
 static struct pm_clk_notifier_block platform_domain_notifier = {
.pm_domain = _pm_domain,
+   .clks = { ERR_PTR(-EAGAIN) },
 };
 
 static const struct of_device_id of_keystone_table[] = {
diff --git a/arch/arm/mach-omap1/pm_bus.c b/arch/arm/mach-omap1/pm_bus.c
index 667c163..5506453 100644
--- a/arch/arm/mach-omap1/pm_bus.c
+++ b/arch/arm/mach-omap1/pm_bus.c
@@ -31,6 +31,7 @@ static struct dev_pm_domain default_pm_domain = {
 static struct pm_clk_notifier_block platform_bus_notifier = {
.pm_domain = _pm_domain,
.con_ids = { "ick", "fck", NULL, },
+   .clks = { ERR_PTR(-EAGAIN), ERR_PTR(-EAGAIN) },
 };
 
 static int __init omap1_pm_runtime_init(void)
diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 652b5a3..26f0dcf 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -407,40 +407,6 @@ int pm_clk_runtime_resume(struct device *dev)
 #else /* !CONFIG_PM */
 
 /**
- * enable_clock - Enable a device clock.
- * @dev: Device whose clock is to be enabled.
- * @con_id: Connection ID of the clock.
- */
-static void enable_clock(struct device *dev, const char *con_id)
-{
-   struct clk *clk;
-
-   clk = clk_get(dev, con_id);
-   if (!IS_ERR(clk)) {
-   clk_prepare_enable(clk);
-   clk_put(clk);
-   dev_info(dev, "Runtime PM disabled, clock forced on.\n");
-   }
-}
-
-/**
- * disable_clock - Disable a device clock.
- * @dev: Device whose clock is to be disabled.
- * @con_id: Connection ID of the clock.
- */
-static void disable_clock(struct device *dev, const char *con_id)
-{
-   struct clk *clk;
-
-   clk = clk_get(dev, con_id);
-   if (!IS_ERR(clk)) {
-   clk_disable_unp

Re: [GIT PULL] clk: ti: clock driver code migration to drivers

[Michael Turquette]

Quoting Tony Lindgren (2015-07-14 03:40:06)
 * Tero Kristo t-kri...@ti.com [150714 03:34]:
  On 07/14/2015 12:54 PM, Tony Lindgren wrote:
  * Tero Kristo t-kri...@ti.com [150714 01:56]:
  
  This pull request contains the TI clock driver set to move the clock
  implementations under clock driver. Some small portions of the clock 
  driver
  code still remain under mach-omap2 after this, it should be decided 
  whether
  this code is now obsolete and should be deleted or should someone try to 
  fix
  it.
  
  Hmm care to clarify what is obsolete or broken after this series?
  
  Not after this series, was broken/obsolete already before.
  
  A couple of omap2/omap3 specific clock files still remain under mach-omap2,
  they are DVFS related. OMAP3 core dvfs support is currently completely
  unused (this could probably be removed, or shall we re-introduce the painful
  core dvfs at some point again?), and parts of the omap2 core dpll handling
  code should probably be re-written; or at least verified that it actually
  works properly. I can't test OMAP2 DVFS myself so don't dare to fiddle with
  it I could probably try to get some sort of DVFS test case to work on
  the board farm OMAP2 board I have access to though, I can investigate this.
 
 People seem to still want the 1 GiHz support, but I think that only
 depends on the SmartReflex and some kind of replacement for
 voltagedomains. So if the core DVFS support is unused, I doubt it's
 very high on anybody's list right now.

As long as the default is to have core running at the fast OPP then 1GHz
should be fine. There is a voltage domain dependency from ARM-CORE when
the ARM runs fast at 1GHZ: it requires core to run at the highest OPP as
well.

Regards,
Mike

  
  And I take it's not obsolete or broken because of this series? :)
  
  No, this series does not touch the above mentioned pieces of code, so this
  definitely should not break anything. :)
 
 OK thanks for confirming that.
 
 Regards,
 
 Tony
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm

[Michael Turquette]

Quoting Vignesh R (2015-06-03 04:51:23)
 tbclk is needed by ehrpwm to generate pwm waveforms. Hence, register
 the required clock information.
 
 Signed-off-by: Vignesh R vigne...@ti.com

Looks good to me. Please feel free to add:

Acked-by: Michael Turquette mturque...@baylibre.com

Regards,
Mike

 ---
  drivers/clk/ti/clk-7xx.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
 index 5d2217ae4478..366be43d15fe 100644
 --- a/drivers/clk/ti/clk-7xx.c
 +++ b/drivers/clk/ti/clk-7xx.c
 @@ -304,6 +304,9 @@ static struct ti_dt_clk dra7xx_clks[] = {
 DT_CLK(4882a000.timer, timer_sys_ck, timer_sys_clk_div),
 DT_CLK(4882c000.timer, timer_sys_ck, timer_sys_clk_div),
 DT_CLK(4882e000.timer, timer_sys_ck, timer_sys_clk_div),
 +   DT_CLK(4843e200.ehrpwm, tbclk, ehrpwm0_tbclk),
 +   DT_CLK(48440200.ehrpwm, tbclk, ehrpwm1_tbclk),
 +   DT_CLK(48442200.ehrpwm, tbclk, ehrpwm2_tbclk),
 DT_CLK(NULL, sys_clkin, sys_clkin1),
 { .node_name = NULL },
  };
 -- 
 2.4.1
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 00/27] ARM: OMAP2+: clock code move under clk driver

[Michael Turquette]

Quoting Tero Kristo (2015-06-03 05:33:46)
 On 05/28/2015 02:15 AM, Tony Lindgren wrote:
  * Tero Kristo t-kri...@ti.com [150527 11:32]:
  On 05/26/2015 07:39 PM, Felipe Balbi wrote:
  On Tue, May 26, 2015 at 09:32:16AM -0700, Tony Lindgren wrote:
  * Tony Lindgren t...@atomide.com [150526 09:08]:
  * Tero Kristo t-kri...@ti.com [150525 08:01]:
  Hi,
 
  As requested, posting v3 with somewhat changed diff parameters and
  diffstat attached. Just some minor Makefile changes compared to v2,
  these were discussed under that set.
 
  Set has been pushed to:
  - tree: https://github.com/t-kristo/linux-pm.git
  - branch: for-4.2/ti-clk-move
 
  Looks like this causes a build error for at least omap2 only .config:
 
  drivers/clk/ti/dpll3xxx.o:(.rodata+0x1c): multiple definition of 
  `clkhwops_omap3_dpll'
  drivers/clk/ti/dpll.o:(.rodata+0x0): first defined here
 
  You may want to create a file selecting ARCH_OMAP2PLUS=y, then point
  KCONFIG_ALLCONFIG to that file for make randconfig. Then just build
  randconfigs :) Usually the issues like this are exposed within few
  randconfig builds, some take longer if the options have dependencies.
 
  alternatively, just clone the repository at [1] and use the example
  script provided in README.md.
 
  [1] https://github.com/felipebalbi/omap-seeds
 
 
  Ok, I pushed an updated branch named: for-4.2/ti-clk-move-v4
 
  This definitely compiles with OMAP2 / OMAP3 / OMAP4 / OMAP5 / DRA7 / AM33xx
  / AM43xx only setups (tried it out.)
 
  Thanks yeah seems to work for me now.
 
  Regards,
 
  Tony
 
 
 Question to Mike / Stephen, any chance of getting this in during the 4.2 
 merge anymore seeing we are already at 4.1-rc6?
 
 I can send a pull request if yes. Otherwise I just wait until we are 
 past the next merge.

Hi Tero,

I'd like more time for any regressions this introduces to be fixed, so
lets push back to next merge window. The always-wrong-but-never-by-much
crystal ball[0] predicts June 14. This is less than two weeks away, so
the wait should be short.

[0] http://phb-crystal-ball.org/

Thanks,
Mike

 
 -Tero
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 10/10] CLK: TI: always enable DESHDCP clock

[Michael Turquette]

Quoting Tomi Valkeinen (2015-05-06 03:08:58)
 DESHDCP clock is needed on DRA7 based SoCs to enable the DSS IP. That
 clock is an odd one, as it is not supposed to be any kind of core clock
 for DSS, and we don't even support HDCP, but the clock is still needed
 even for the HWMOD framework to be able to reset the DSS IP.
 
 As there's no support for multiple core clocks in the HWMOD framework,
 we don't have any obvious place to enable this clock when DSS IP is
 being enabled.
 
 Furthermore, the HDMI on OMAP5 DSS is the same as on DRA7, and OMAP5
 does not have any such clock configuration bit. This suggests that on
 OMAP5 the DESHDCP clock is always enabled, and for DRA7 we have the
 possibility to gate it.
 
 So, as we don't have any clean way to enable and disable the clock
 based on the need, this patch enables the clock at boot time, making it
 work similarly to OMAP5.
 
 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com

Acked-by: Michael Turquette mturque...@linaro.org

 ---
  drivers/clk/ti/clk-7xx.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
 index 2dd956b9affa..63b8323df918 100644
 --- a/drivers/clk/ti/clk-7xx.c
 +++ b/drivers/clk/ti/clk-7xx.c
 @@ -312,7 +312,7 @@ static struct ti_dt_clk dra7xx_clks[] = {
  int __init dra7xx_dt_clk_init(void)
  {
 int rc;
 -   struct clk *abe_dpll_mux, *sys_clkin2, *dpll_ck;
 +   struct clk *abe_dpll_mux, *sys_clkin2, *dpll_ck, *hdcp_ck;
  
 ti_dt_clocks_register(dra7xx_clks);
  
 @@ -348,5 +348,10 @@ int __init dra7xx_dt_clk_init(void)
 if (rc)
 pr_err(%s: failed to set USB_DPLL M2 OUT\n, __func__);
  
 +   hdcp_ck = clk_get_sys(NULL, dss_deshdcp_clk);
 +   rc = clk_prepare_enable(hdcp_ck);
 +   if (rc)
 +   pr_err(%s: failed to set dss_deshdcp_clk\n, __func__);
 +
 return rc;
  }
 -- 
 2.1.4
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 10/10] CLK: TI: always enable DESHDCP clock

[Michael Turquette]

Quoting Stephen Boyd (2015-05-20 12:34:23)
 On 05/20/15 04:50, Tero Kristo wrote:
 
 
  @@ -348,5 +348,10 @@ int __init dra7xx_dt_clk_init(void)
if (rc)
pr_err(%s: failed to set USB_DPLL M2 OUT\n, __func__);
 
  +hdcp_ck = clk_get_sys(NULL, dss_deshdcp_clk);
  +rc = clk_prepare_enable(hdcp_ck);
  +if (rc)
  +pr_err(%s: failed to set dss_deshdcp_clk\n, __func__);
  +
return rc;
}
 
 
  You should rather use the assigned-clock properties in DT to accomplish
  this, the manual clock tweaks under the drivers/clk/ti/clk-* files
  should be converted to DT setup also.
 
  Now that I sent this, I realize we only have support to set_parent /
  set_rate through the assigned-clock props, no enable. Any plans to
  extend this support Mike/Stephen?
 
 
 
 Enable falls under the critical clocks discussion that is ongoing. I
 assume that this is some sort of critical clock that can't be turned off?

Just chiming in on the critical clock discussion. I'm not planning to
merge something that lets Devicetree nodes call clk_enable on a clock.
That's what drivers are for.

The assigned-rate and assigned-parent stuff that Tero mentioned is more
like configuration data for a downstream clock consumer. Clock
gating/ungating does not fall under this type of configuration data in
my opinion.

I think that Tomi's patch to call clk_prepare_enable from
dra7xx_dt_clk_init is a reasonable solution to the problem.

Regards,
Mike

 
 -- 
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: AM335x OMAP2 common clock external fixed-clock registration

[Michael Turquette]

Quoting Russell King - ARM Linux (2015-04-17 03:18:33)
 On Fri, Apr 17, 2015 at 11:12:03AM +0200, Sebastian Hesselbarth wrote:
  On 17.04.2015 04:00, Michael Welling wrote:
  On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote:
  On 17.04.2015 00:09, Michael Welling wrote:
  On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote:
  On 16.04.2015 18:17, Michael Welling wrote:
  [...]
  What would be the proper error path?
  What cleanup is required?
  
  A proper error path would be to release any claimed resource
  on any error. If you look at the code, the only resources that
  need to be released are the two clocks in question.
  
  So for every error return in the probe function and in the of 
  si5351_dt_parse
  it needs to clk_put first right?
  
  Not quite. The driver should clk_put() every clock that it called a
  [of_]clk_get() for. The thing is that clocks can be passed by
  platform_data and we never claim them.
 
 I've always said clocks (as in struct clk) should never be passed through
 platform data.

+1

And for ccf clock drivers Stephen and I plan to change the behavior of
clk_register() at some point so that it returns an error code and not a
struct clk. This will make clk_dev the only way to get at a struct clk
for users of the ccf implementation.

Of course it is still possible to clk_get from some place and pass as
platform_data, but every little bit helps.

Regards,
Mike

 
 -- 
 FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
 according to speedtest.net.
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] OMAP2+: TI clock driver changes for 4.1 merge window

[Michael Turquette]

Quoting Tero Kristo (2015-03-24 12:06:41)
 Hi Mike,
 
 Here are the TI clock driver patches meant for 4.1 merge window.
 
 Some simple data fixes, dm81xx FAPLL changes, and a generic TI clock 
 driver register access fix taken from the PRCM/SCM set I have posted to 
 linux-omap list earlier.

Pulled.

Thanks,
Mike

 
 -Tero
 
 
 
 The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539:
 
Linux 4.0-rc1 (2015-02-22 18:21:14 -0800)
 
 are available in the git repository at:
 
g...@github.com:t-kristo/linux-pm.git for-4.1-clk-ti
 
 for you to fetch changes up to 9089848d9afa34a796988b5b666c2c4e611ccb61:
 
clk: ti: Implement FAPLL set_rate for the PLL (2015-03-24 20:26:14 +0200)
 
 
 Peter Ujfalusi (2):
clk: ti: clk-3xxx: Correct McBSP related DT clock definitions
clk: ti: clk-3xxx-legacy: Correct McBSP related clock aliases
 
 Suman Anna (4):
clk: ti: OMAP4: Remove the legacy timer DT clock aliases
clk: ti: OMAP5: Correct the DT clock aliases for timers
clk: ti: DRA7: Correct timer_sys_ck clock aliases for Timers
clk: ti: DRA7: Add timer_sys_ck aliases for Timers 13 through 16
 
 Tero Kristo (1):
clk: ti: fix ti_clk_get_reg_addr error handling
 
 Tony Lindgren (3):
clk: ti: Fix FAPLL recalc_rate for rounding errors
clk: ti: Implement FAPLL set_rate for the synthesizer
clk: ti: Implement FAPLL set_rate for the PLL
 
   drivers/clk/ti/apll.c|5 +-
   drivers/clk/ti/autoidle.c|2 +-
   drivers/clk/ti/clk-3xxx-legacy.c |   16 ++-
   drivers/clk/ti/clk-3xxx.c|   19 +--
   drivers/clk/ti/clk-44xx.c|   11 --
   drivers/clk/ti/clk-54xx.c|   22 ++--
   drivers/clk/ti/clk-7xx.c |   18 ++-
   drivers/clk/ti/clk.c |7 +-
   drivers/clk/ti/divider.c |4 +-
   drivers/clk/ti/dpll.c|6 +-
   drivers/clk/ti/fapll.c   |  270 
 --
   drivers/clk/ti/gate.c|4 +-
   drivers/clk/ti/interface.c   |2 +-
   drivers/clk/ti/mux.c |4 +-
   14 files changed, 318 insertions(+), 72 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 32/35 linux-next] clk: constify of_device_id array

[Michael Turquette]

Quoting Fabian Frederick (2015-03-16 12:59:06)
 of_device_id is always used as const.
 (See driver.of_match_table and open firmware functions)
 
 Signed-off-by: Fabian Frederick f...@skynet.be

Acked-by: Michael Turquette mturque...@linaro.org

 ---
  drivers/clk/clk-palmas.c | 2 +-
  drivers/clk/st/clkgen-fsyn.c | 2 +-
  drivers/clk/st/clkgen-mux.c  | 8 
  drivers/clk/st/clkgen-pll.c  | 4 ++--
  drivers/clk/ti/clk-dra7-atl.c| 2 +-
  drivers/clk/ti/clockdomain.c | 2 +-
  drivers/clk/versatile/clk-vexpress-osc.c | 2 +-
  7 files changed, 11 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/clk/clk-palmas.c b/drivers/clk/clk-palmas.c
 index 8d45992..45a535a 100644
 --- a/drivers/clk/clk-palmas.c
 +++ b/drivers/clk/clk-palmas.c
 @@ -161,7 +161,7 @@ static struct palmas_clks_of_match_data 
 palmas_of_clk32kgaudio = {
 },
  };
  
 -static struct of_device_id palmas_clks_of_match[] = {
 +static const struct of_device_id palmas_clks_of_match[] = {
 {
 .compatible = ti,palmas-clk32kg,
 .data = palmas_of_clk32kg,
 diff --git a/drivers/clk/st/clkgen-fsyn.c b/drivers/clk/st/clkgen-fsyn.c
 index af94ed8..a917c4c 100644
 --- a/drivers/clk/st/clkgen-fsyn.c
 +++ b/drivers/clk/st/clkgen-fsyn.c
 @@ -1057,7 +1057,7 @@ static struct clk * __init 
 st_clk_register_quadfs_fsynth(
 return clk;
  }
  
 -static struct of_device_id quadfs_of_match[] = {
 +static const struct of_device_id quadfs_of_match[] = {
 {
 .compatible = st,stih416-quadfs216,
 .data = st_fs216c65_416
 diff --git a/drivers/clk/st/clkgen-mux.c b/drivers/clk/st/clkgen-mux.c
 index 9a15ec3..fdcff10 100644
 --- a/drivers/clk/st/clkgen-mux.c
 +++ b/drivers/clk/st/clkgen-mux.c
 @@ -341,7 +341,7 @@ static struct clkgena_divmux_data st_divmux_c32odf3 = {
 .fb_start_bit_idx = 24,
  };
  
 -static struct of_device_id clkgena_divmux_of_match[] = {
 +static const struct of_device_id clkgena_divmux_of_match[] = {
 {
 .compatible = st,clkgena-divmux-c65-hs,
 .data = st_divmux_c65hs,
 @@ -479,7 +479,7 @@ static struct clkgena_prediv_data prediv_c32_data = {
 .table = prediv_table16,
  };
  
 -static struct of_device_id clkgena_prediv_of_match[] = {
 +static const struct of_device_id clkgena_prediv_of_match[] = {
 { .compatible = st,clkgena-prediv-c65, .data = prediv_c65_data },
 { .compatible = st,clkgena-prediv-c32, .data = prediv_c32_data },
 {}
 @@ -586,7 +586,7 @@ static struct clkgen_mux_data stih407_a9_mux_data = {
 .width = 2,
  };
  
 -static struct of_device_id mux_of_match[] = {
 +static const struct of_device_id mux_of_match[] = {
 {
 .compatible = st,stih416-clkgenc-vcc-hd,
 .data = clkgen_mux_c_vcc_hd_416,
 @@ -693,7 +693,7 @@ static struct clkgen_vcc_data st_clkgenf_vcc_416 = {
 .lock = clkgenf_lock,
  };
  
 -static struct of_device_id vcc_of_match[] = {
 +static const struct of_device_id vcc_of_match[] = {
 { .compatible = st,stih416-clkgenc, .data = st_clkgenc_vcc_416 },
 { .compatible = st,stih416-clkgenf, .data = st_clkgenf_vcc_416 },
 {}
 diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c
 index 29769d7..d204ba8 100644
 --- a/drivers/clk/st/clkgen-pll.c
 +++ b/drivers/clk/st/clkgen-pll.c
 @@ -593,7 +593,7 @@ static struct clk * __init clkgen_odf_register(const char 
 *parent_name,
 return clk;
  }
  
 -static struct of_device_id c32_pll_of_match[] = {
 +static const struct of_device_id c32_pll_of_match[] = {
 {
 .compatible = st,plls-c32-a1x-0,
 .data = st_pll3200c32_a1x_0,
 @@ -708,7 +708,7 @@ err:
  }
  CLK_OF_DECLARE(clkgen_c32_pll, st,clkgen-plls-c32, clkgen_c32_pll_setup);
  
 -static struct of_device_id c32_gpu_pll_of_match[] = {
 +static const struct of_device_id c32_gpu_pll_of_match[] = {
 {
 .compatible = st,stih415-gpu-pll-c32,
 .data = st_pll1200c32_gpu_415,
 diff --git a/drivers/clk/ti/clk-dra7-atl.c b/drivers/clk/ti/clk-dra7-atl.c
 index 59bb4b3..d86bc46 100644
 --- a/drivers/clk/ti/clk-dra7-atl.c
 +++ b/drivers/clk/ti/clk-dra7-atl.c
 @@ -294,7 +294,7 @@ static int of_dra7_atl_clk_remove(struct platform_device 
 *pdev)
 return 0;
  }
  
 -static struct of_device_id of_dra7_atl_clk_match_tbl[] = {
 +static const struct of_device_id of_dra7_atl_clk_match_tbl[] = {
 { .compatible = ti,dra7-atl, },
 {},
  };
 diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c
 index b4c5fac..35fe108 100644
 --- a/drivers/clk/ti/clockdomain.c
 +++ b/drivers/clk/ti/clockdomain.c
 @@ -52,7 +52,7 @@ static void __init of_ti_clockdomain_setup(struct 
 device_node *node)
 }
  }
  
 -static struct of_device_id ti_clkdm_match_table[] __initdata = {
 +static const struct

[PATCH 1/3] arm: omap2+ remove dead clock code

[Michael Turquette]

Remove omap_clocks_register and dummy_ck. The former is not used anymore
now that the statically defined clk stuctures are replaced with proper
descriptors and registered with the framework.

The dummy clock in arch/arm/mach-omap2 is made redundant by the OMAP3+
clock data that migrated to drivers/clk.

An additional benefit to this clean-up is removing the references to
clk-private.h which will be removed.

Cc: Paul Walmsley p...@pwsan.com
Cc: Tero Kristo t-kri...@ti.com
Cc: Tony Lindgren t...@atomide.com
Signed-off-by: Michael Turquette mturque...@linaro.org
---
Cc: linux-omap@vger.kernel.org
 arch/arm/mach-omap2/clock.c | 16 
 arch/arm/mach-omap2/clock.h |  5 -
 arch/arm/mach-omap2/clock_common_data.c | 14 --
 3 files changed, 35 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 6ad5b4d..d9c128e 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -23,7 +23,6 @@
 #include linux/clk-provider.h
 #include linux/io.h
 #include linux/bitops.h
-#include linux/clk-private.h
 #include asm/cpu.h
 
 #include trace/events/power.h
@@ -630,21 +629,6 @@ const struct clk_hw_omap_ops clkhwops_wait = {
 };
 
 /**
- * omap_clocks_register - register an array of omap_clk
- * @ocs: pointer to an array of omap_clk to register
- */
-void __init omap_clocks_register(struct omap_clk oclks[], int cnt)
-{
-   struct omap_clk *c;
-
-   for (c = oclks; c  oclks + cnt; c++) {
-   clkdev_add(c-lk);
-   if (!__clk_init(NULL, c-lk.clk))
-   omap2_init_clk_hw_omap_clocks(c-lk.clk);
-   }
-}
-
-/**
  * omap2_clk_switch_mpurate_at_boot - switch ARM MPU rate by boot-time argument
  * @mpurate_ck_name: clk name of the clock to change rate
  *
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index c5b3a7f..6a10ce3 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -245,7 +245,6 @@ struct ti_clk_features {
 extern struct ti_clk_features ti_clk_features;
 
 extern const struct clkops clkops_omap2_dflt_wait;
-extern const struct clkops clkops_dummy;
 extern const struct clkops clkops_omap2_dflt;
 
 extern struct clk_functions omap2_clk_functions;
@@ -254,8 +253,6 @@ extern const struct clksel_rate gpt_32k_rates[];
 extern const struct clksel_rate gpt_sys_rates[];
 extern const struct clksel_rate gfx_l3_rates[];
 extern const struct clksel_rate dsp_ick_rates[];
-extern struct clk_core dummy_ck_core;
-extern struct clk dummy_ck;
 
 extern const struct clk_hw_omap_ops clkhwops_iclk_wait;
 extern const struct clk_hw_omap_ops clkhwops_wait;
@@ -280,7 +277,5 @@ extern void __iomem *clk_memmaps[];
 extern int omap2_clkops_enable_clkdm(struct clk_hw *hw);
 extern void omap2_clkops_disable_clkdm(struct clk_hw *hw);
 
-extern void omap_clocks_register(struct omap_clk *oclks, int cnt);
-
 void __init ti_clk_init_features(void);
 #endif
diff --git a/arch/arm/mach-omap2/clock_common_data.c 
b/arch/arm/mach-omap2/clock_common_data.c
index febd0a2..61b60df 100644
--- a/arch/arm/mach-omap2/clock_common_data.c
+++ b/arch/arm/mach-omap2/clock_common_data.c
@@ -16,7 +16,6 @@
  * OMAP3xxx clock definition files.
  */
 
-#include linux/clk-private.h
 #include clock.h
 
 /* clksel_rate data common to 24xx/343x */
@@ -114,16 +113,3 @@ const struct clksel_rate div31_1to31_rates[] = {
{ .div = 31, .val = 31, .flags = RATE_IN_4430 | RATE_IN_AM33XX },
{ .div = 0 },
 };
-
-/* Clocks shared between various OMAP SoCs */
-
-static struct clk_ops dummy_ck_ops = {};
-
-struct clk_core dummy_ck_core = {
-   .name = dummy_clk,
-   .ops = dummy_ck_ops,
-   .flags = CLK_IS_BASIC,
-};
-struct clk dummy_ck = {
-   .core = dummy_ck_core,
-};
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html